Persona is no longer an option for authentication on BMO. For more details see Persona Deprecated.
Last Comment Bug 725674 - IonMonkey: Compile JSOP_LAMBDA and JSOP_DEFLOCALFUN
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Jan de Mooij [:jandem]
: Jason Orendorff [:jorendorff]
Depends on: 797185
Blocks: 684381
  Show dependency treegraph
Reported: 2012-02-09 08:34 PST by Jan de Mooij [:jandem]
Modified: 2012-10-02 16:00 PDT (History)
2 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (38.69 KB, patch)
2012-02-10 04:39 PST, Jan de Mooij [:jandem]
dvander: review+
Details | Diff | Splinter Review

Description Jan de Mooij [:jandem] 2012-02-09 08:34:42 PST

Comment 1 Jan de Mooij [:jandem] 2012-02-10 04:39:17 PST
Created attachment 596017 [details] [diff] [review]

JSOP_LAMBDA is one of the more complicated deffun*/lambda* ops because we can do some optimizations to avoid cloning the function object in some cases, I hope the other ops will be a bit simpler.
Comment 2 David Anderson [:dvander] 2012-02-10 14:52:32 PST
Comment on attachment 596017 [details] [diff] [review]

Review of attachment 596017 [details] [diff] [review]:

::: js/src/ion/IonBuilder.cpp
@@ +3515,5 @@
> +IonBuilder::jsop_lambda(JSFunction *fun)
> +{
> +    JS_ASSERT(script->analysis()->usesScopeChain());
> +    current->pushSlot(info().scopeChainSlot());
> +    MDefinition *scopeChain = current->pop();

It should be okay to use getSlot here, if you want to.

::: js/src/jsanalyze.cpp
@@ +344,5 @@
> +          case JSOP_DEFLOCALFUN:
> +          case JSOP_LAMBDA:
> +            usesScopeChain_ = true;
> +            isInlineable = false;

Why didn't JM need this to set usesScopeChain?
Comment 3 Jan de Mooij [:jandem] 2012-02-11 05:54:51 PST

(In reply to David Anderson [:dvander] from comment #2)
> It should be okay to use getSlot here, if you want to.


> Why didn't JM need this to set usesScopeChain?

JM's lambda stubs call StackFrame::scopeChain(), which sets the scope chain to the callee's environment if HAS_SCOPECHAIN is not set. I discussed this with bhackett on IRC, there are two ways to determine the parent of the new function in IM:

1) Use the scope chain slot (and set usesScopChain_ to true)
2) Use LFunctionEnvironment.

According to bhackett a good argument for 1) is that it will be forward compatible with block compilation.

Note You need to log in before you can comment on or make changes to this bug.