[webkit-reviews] review denied: [Bug 130389] Remove ENABLE(LLINT) and ENABLE(LLINT_C_LOOP) guards : [Attachment 232608] Proposed patch

[bugzilla-daemon at webkit.org]

Mark Lam <mark.lam at apple.com> has denied Dániel Bátyai
<dbatyai.u-szeged at partner.samsung.com>'s request for review:
Bug 130389: Remove ENABLE(LLINT) and ENABLE(LLINT_C_LOOP) guards
https://bugs.webkit.org/show_bug.cgi?id=130389

Attachment 232608: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=232608&action=review

------- Additional Comments from Mark Lam <mark.lam at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=232608&action=review


Almost there, but need a few more changes.  Please rebase to ToT while you're
at it.	Thanks.

> Source/JavaScriptCore/CMakeLists.txt:628
>      )
>      target_link_libraries(LLIntOffsetsExtractor WTF)

You unindented everything else in this previous "if (ENABLE_LLINT)" section,
but left the above block indented.  Is there a reason for this?

> Source/JavaScriptCore/interpreter/JSStack.cpp:47
> +#endif // ENABLE(JIT)

I think this should match the #if above i.e. "#endif // !ENABLE(JIT)".	Same
with all the other cases above and below if a comment is needed after the
#endif.

> Source/JavaScriptCore/interpreter/JSStack.cpp:66
> +#endif // ENABLE(JIT)

Ditto.

> Source/JavaScriptCore/interpreter/JSStack.cpp:161
> +#endif // ENABLE(JIT)

Ditto.

> Source/JavaScriptCore/interpreter/JSStack.h:123
> +#endif // ENABLE(JIT)

Ditto.

> Source/JavaScriptCore/llint/LLIntOpcode.h:34
> +#else // ENABLE(JIT) !cloop

After reading this, I think it's better to remove all the "cloop" and "!cloop"
comments in the patch.	After all, ENABLE(JIT) implies !cloop and vice versa. 
Hence, there's no info gained by having all the "cloop" comments.  Can you
remove them please?