[webkit-reviews] review denied: [Bug 233949] [WTF] RELEASE_ASSERT_WITH_MESSAGE() should always print the message : [Attachment 450711] RFC Patch

[bugzilla-daemon at webkit.org]

Michael Catanzaro <mcatanzaro at gnome.org> has denied Adrian Perez
<aperez at igalia.com>'s request for review:
Bug 233949: [WTF] RELEASE_ASSERT_WITH_MESSAGE() should always print the message
https://bugs.webkit.org/show_bug.cgi?id=233949

Attachment 450711: RFC Patch

https://bugs.webkit.org/attachment.cgi?id=450711&action=review




--- Comment #15 from Michael Catanzaro <mcatanzaro at gnome.org> ---
Comment on attachment 450711
  --> https://bugs.webkit.org/attachment.cgi?id=450711
RFC Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=450711&action=review

OK, I like how you fixed RELEASE_ASSERT_WITH_MESSAGE. This is clearly the way
to go IMO. The original RELEASE_ASSERT_WITH_MESSAGE() was just broken.

But I'm not sure what you're doing with this VERBOSE_RELEASE_ASSERT().

P.S. I removed [WTF] from the title of this bug because I've never seen that
tag used before.

> Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:57
> -#undef RELEASE_ASSERT
> -#define RELEASE_ASSERT(assertion) do { \
> -    if (!(assertion)) { \
> +#define VERBOSE_RELEASE_ASSERT(assertion, ...) do { \
> +    if (UNLIKELY(!(assertion))) { \
>	   WTFReportAssertionFailure(__FILE__, __LINE__, WTF_PRETTY_FUNCTION,
#assertion); \
> -	   CRASH(); \
> +	   CRASH_WITH_INFO(__VA_ARGS__); \
>      } \
> -} while (0)
> +} while (false)

I don't understand: why not get rid of this, and change the callers to use your
fixed RELEASE_ASSERT_WITH_MESSAGE()?