[webkit-reviews] review granted: [Bug 115542] Rationalized 'this' value conversion : [Attachment 200385] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has granted Geoffrey Garen <ggaren at apple.com>'s
request for review:
Bug 115542: Rationalized 'this' value conversion
https://bugs.webkit.org/show_bug.cgi?id=115542

Attachment 200385: Patch
https://bugs.webkit.org/attachment.cgi?id=200385&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=200385&action=review


Do you want to land on trunk?  Or on the branch?  R=me either way, but I sort
of prefer the branch.

> Source/JavaScriptCore/dfg/DFGFixupPhase.cpp:639
> +	       ECMAMode ecmaMode =
m_graph.executableFor(node->codeOrigin)->isStrictMode() ? StrictMode :
NotStrictMode;

Oh thank god for the ECMAMode enum.

> Source/JavaScriptCore/llint/LLIntData.cpp:107
> +    ASSERT(FinalObjectType == 18);

Is this just to have a fast "am I already suitable for this" check?

I buy that it's probably a sensible way to do it, but I do wonder why not have
a type flag instead.

(I'm not objecting.  I'm just curious what your thinking was.)