ecma262 icon indicating copy to clipboard operation
ecma262 copied to clipboard

Consider dropping the "optimization" for functions without parameter default value initializers

Open jorendorff opened this issue 8 years ago • 5 comments

Currently, FunctionDeclarationInstantiation branches at step 26:

26. If hasParameterExpressions is false, then
    a.  NOTE Only a single lexical environment is needed for the parameters
        and top-level vars.
    ...
27. Else,
    a.  NOTE A separate Environment Record is needed to ensure that closures
        created by expressions in the formal parameter list do not have visibility
        of declarations in the function body.
    ...

There's a special case (step 26) for functions without defaults; and a general case (step 27) that looks like it would work whether you have defaults or not.

Our implementation will in fact branch like this, somewhere. But why have the special case in the spec? It reads like a pure optimization, which seems undesirable for a spec. I think @allenwb wrote this - Allen, do you know what this is for?

jorendorff avatar Jun 28 '16 17:06 jorendorff

I considered that possibility when I updated the algorithm to include the arguments environment. The reason I didn't go that route is that I wanted to emphasize that the separate environment was there solely to support parameter lists with defaults. Plus the overall algorithm is complicated enough that it seemed wise to normatively specify the intended semantics for the "optimized" case rather than leaving it to each implementation to figure that out.

The optimized branch could be eliminated and replaced with an informative note. However, at the time, carefully crafting the language of such a note seemed harder than just including both paths in the algorithm.

allenwb avatar Jun 28 '16 18:06 allenwb

Plus the overall algorithm is complicated enough that it seemed wise to normatively specify the intended semantics for the "optimized" case rather than leaving it to each implementation to figure that out.

Thanks for thinking of us implementors, and thanks for the fast response.

The NOTE in step 26.a. should say that this is meant to illustrate a pure optimization that implementations may do. As it stands, I was concerned that the branch might have been done, rather, because the two branches were not the same, that adding even a trivial default like = 0 changed behavior in some very subtle way that we were missing.

Honestly, though, I think I'd rather this be removed. Implementations will likely choose to optimize this some other way. For example, nested environments can often be fused; this particular optimization might fall out for free, as a special case of a more general analysis that looks for such opportunities.

jorendorff avatar Jun 28 '16 20:06 jorendorff

I like @jorendorff 's suggestion. Our implementation has a conditional too, but we only add the scope if we see sloppy direct eval being called anywhere in the function body or defaults.

littledan avatar Jun 29 '16 20:06 littledan

@jorendorff any interest in making a PR?

ljharb avatar Jan 03 '20 04:01 ljharb

Apart from this, there's also the argumentsObjectNeeded stuff, which is unobservable (except that you do need to avoid creating the binding for arrow functions, of course).

Similarly, the condition containing

NOTE: Non-strict functions use a separate Environment Record for top-level lexical declarations so that a direct eval can determine whether any var scoped declarations introduced by the eval code conflict with pre-existing top-level lexically scoped declarations. This is not needed for strict functions because a strict direct eval always places all declarations into a new Environment Record.

isn't observable; it is equivalent to always create the new environment.

bakkot avatar Aug 26 '21 06:08 bakkot