Implement B.3.5 (Closes #548)

[adrianheine]

Spec: B.3.5

I'm not sure I actually got the semantics right, test262 unfortunately doesn't include any specific negative tests for B.3.5.

There's definitely a problem with function declarations in the catch block, but that's present in current trunk as well. As far as I understand it, function declarations should be handled as lexical bindings, but B.3.3.1 allows them to re-declare each other (as in if (a) function f() {} else function f() {}). Currently, they are handled as var bindings, which makes them behave wrongly in catch blocks.

[adrianheine]

Thanks for your feedback, it's really hard for me to understand the spec. My understanding is that try {} catch (foo) { function foo() {} } should pass due to B.3.3.1 and B.3.5 interacting with each other. B.3.3.1 would turn the FunctionDeclaration into a VariableStatement and B.3.5 triggers for VariableStatements if the CatchParameter is BindingIdentifier, which is also true.

Your other code examples should fail imho because CatchParameter is not BindingIdentifier, so »It is a Syntax Error if any element of the BoundNames of CatchParameter also occurs in the VarDeclaredNames of Block«.

[adrianheine]

try {} catch(e) { function e(){} }: I compared this to
test/annexB/language/function-code/if-stmt-else-decl-func-no-skip-try.js and found that it should behave the same (i. e. be valid) since the if doesn't create a new scope.

try {} catch(e) { let e; }: Correctly fails

try {} catch(e) { for(var e of 0); }: Already fails

[marijnh]

BTW. Why not lazily create block prototypes?

The scope tracking code doesn't appear to have been written with performance in mind, and it would definitely be valuable is someone revisits that to be more efficient.

[adrianheine]

Hey @KFlash, I really appreciate you giving me feedback here since I'm pretty new to the ECMAScript spec. However, I have a hard time actually learning something from the feedback, because you don't respond to my answers but instead add new suggestions. If my answers above show some misunderstanding of the spec, I would really like you to follow up on them specifically and challenge my understanding so that I can learn something.

[adrianheine]

That's what I thought, too, but the ECMAScript test suite test262 has a test with the following code:

  try {
    throw null;
  } catch (f) {

  if (false) ; else function f() { return 123; }

}

I don't understand how this should be different than your example.

[RReverser]

@adrianheine This is different because function declaration in if statements is allowed separately by B.3.4.

[RReverser]

Of course, but that test is explicitly not in strict mode :)

[adrianheine]

I understand that function declaration in if statements is allowed in B.3.4. The question here is how a function declaration as consequent or alternate of an if statement behaves in terms of scoping. I suppose the sentence »Code matching this production is processed as if each matching occurrence of FunctionDeclaration[?Yield, ?Await, ~Default] was the sole StatementListItem of a BlockStatement occupying that position in the source code.« from B.3.4 is relevant for this, right? Unfortunately, as I pointed out in the initial post of this pull request, acorn currently parses functions as var bindings, not let bindings. While it's a bug, I suppose that's in order to support B.3.3.1. I'd be willing to look into this as well.

What about try {} catch ([foo]) { var foo; } and try {} catch ({ foo }) { var foo; }? @KFlash wrote in the first answer that these should pass. As I wrote before, my understanding is that the exception from B.3.5 does not trigger since CatchParameter is not BindingIdentifier, so this is a syntax error. Which is right?

[adrianheine]

I would think that with try {} catch ({ foo }) { var foo; }, VarDeclaredNames of Block would be [ foo ], BoundNames of CatchParameter would also be [ foo ] and CatchParameter is CatchParameter:BindingIdentifier would be false, since CatchParameter is BindingPattern, thus

It is a Syntax Error if any element of the BoundNames of CatchParameter also occurs in the VarDeclaredNames of Block unless CatchParameter is CatchParameter:BindingIdentifier and that element is only bound by a VariableStatement, the VariableDeclarationList of a for statement, the ForBinding of a for-in statement, or the BindingIdentifier of a for-in statement. = It is a Syntax Error if any element of [ foo ] also occurs in [ foo ] unless false. = It is a syntax error.

I guess I'm wrong about VarDeclaredNames of Block?

[marijnh]

I've pushed 21e852f...e4108f3 to address this, which seem to at least increase the amount of test262 cases we pass.