[webkit-reviews] review denied: [Bug 96127] JSC: Fix some llint C++ interpreter bugs : [Attachment 162844] Fixed WTF's ASSERT() instead.

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied  review:
Bug 96127: JSC: Fix some llint C++ interpreter bugs
https://bugs.webkit.org/show_bug.cgi?id=96127

Attachment 162844: Fixed WTF's ASSERT() instead.
https://bugs.webkit.org/attachment.cgi?id=162844&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=162844&action=review


> Source/WTF/wtf/Assertions.h:247
> +static ALWAYS_INLINE void wtfAssert(bool assertion, const char* file, int
line,

ALWAYS_INLINE isn't meaningful here. Inlining is turned of in ASSERT builds.
Like I said before, ASSERT is not a performance-relevant code path.

> Source/WTF/wtf/Assertions.h:249
> +				       const char* function,
> +				       const char* assertionString)

WebKit style is not to indent lines like this, because it adds extra busy work
to later edits to keep the lines equally indented. You can either put
"function" and "assertionString" on the same line, indented by one tabstop, or
you can put each of assertion, file, line, function, and assertionString on
their own lines, indented by one tabstop.