Improve handling of scoping for function declarations and catch params

Issue #620, issue #548

This is relatively ad-hoc and weird, but so is annex B.3.5, so maybe
that's okay. It's very possible that I missed some circumstances.

Author: marijnh

bin/test262.whitelist

@@ -1,91 +1,3 @@
-annexB/language/function-code/block-decl-func-no-skip-try.js (default)
-annexB/language/function-code/block-decl-func-skip-early-err-block.js (default)
-annexB/language/function-code/block-decl-func-skip-early-err.js (default)
-annexB/language/function-code/block-decl-func-skip-early-err-switch.js (default)
-annexB/language/function-code/block-decl-func-skip-early-err-try.js (default)
-annexB/language/function-code/if-decl-else-decl-a-func-no-skip-try.js (default)
-annexB/language/function-code/if-decl-else-decl-a-func-skip-early-err-block.js (default)
-annexB/language/function-code/if-decl-else-decl-a-func-skip-early-err.js (default)
-annexB/language/function-code/if-decl-else-decl-a-func-skip-early-err-switch.js (default)
-annexB/language/function-code/if-decl-else-decl-a-func-skip-early-err-try.js (default)
-annexB/language/function-code/if-decl-else-decl-b-func-no-skip-try.js (default)
-annexB/language/function-code/if-decl-else-decl-b-func-skip-early-err-block.js (default)
-annexB/language/function-code/if-decl-else-decl-b-func-skip-early-err.js (default)
-annexB/language/function-code/if-decl-else-decl-b-func-skip-early-err-switch.js (default)
-annexB/language/function-code/if-decl-else-decl-b-func-skip-early-err-try.js (default)
-annexB/language/function-code/if-decl-else-stmt-func-no-skip-try.js (default)
-annexB/language/function-code/if-decl-else-stmt-func-skip-early-err-block.js (default)
-annexB/language/function-code/if-decl-else-stmt-func-skip-early-err.js (default)
-annexB/language/function-code/if-decl-else-stmt-func-skip-early-err-switch.js (default)
-annexB/language/function-code/if-decl-else-stmt-func-skip-early-err-try.js (default)
-annexB/language/function-code/if-decl-no-else-func-no-skip-try.js (default)
-annexB/language/function-code/if-decl-no-else-func-skip-early-err-block.js (default)
-annexB/language/function-code/if-decl-no-else-func-skip-early-err.js (default)
-annexB/language/function-code/if-decl-no-else-func-skip-early-err-switch.js (default)
-annexB/language/function-code/if-decl-no-else-func-skip-early-err-try.js (default)
-annexB/language/function-code/if-stmt-else-decl-func-no-skip-try.js (default)
-annexB/language/function-code/if-stmt-else-decl-func-skip-early-err-block.js (default)
-annexB/language/function-code/if-stmt-else-decl-func-skip-early-err.js (default)
-annexB/language/function-code/if-stmt-else-decl-func-skip-early-err-switch.js (default)
-annexB/language/function-code/if-stmt-else-decl-func-skip-early-err-try.js (default)
-annexB/language/function-code/switch-case-func-no-skip-try.js (default)
-annexB/language/function-code/switch-case-func-skip-early-err-block.js (default)
-annexB/language/function-code/switch-case-func-skip-early-err.js (default)
-annexB/language/function-code/switch-case-func-skip-early-err-switch.js (default)
-annexB/language/function-code/switch-case-func-skip-early-err-try.js (default)
-annexB/language/function-code/switch-dflt-func-no-skip-try.js (default)
-annexB/language/function-code/switch-dflt-func-skip-early-err-block.js (default)
-annexB/language/function-code/switch-dflt-func-skip-early-err.js (default)
-annexB/language/function-code/switch-dflt-func-skip-early-err-switch.js (default)
-annexB/language/function-code/switch-dflt-func-skip-early-err-try.js (default)
-annexB/language/global-code/block-decl-global-no-skip-try.js (default)
-annexB/language/global-code/block-decl-global-skip-early-err-block.js (default)
-annexB/language/global-code/block-decl-global-skip-early-err.js (default)
-annexB/language/global-code/block-decl-global-skip-early-err-switch.js (default)
-annexB/language/global-code/block-decl-global-skip-early-err-try.js (default)
-annexB/language/global-code/if-decl-else-decl-a-global-no-skip-try.js (default)
-annexB/language/global-code/if-decl-else-decl-a-global-skip-early-err-block.js (default)
-annexB/language/global-code/if-decl-else-decl-a-global-skip-early-err.js (default)
-annexB/language/global-code/if-decl-else-decl-a-global-skip-early-err-switch.js (default)
-annexB/language/global-code/if-decl-else-decl-a-global-skip-early-err-try.js (default)
-annexB/language/global-code/if-decl-else-decl-b-global-no-skip-try.js (default)
-annexB/language/global-code/if-decl-else-decl-b-global-skip-early-err-block.js (default)
-annexB/language/global-code/if-decl-else-decl-b-global-skip-early-err.js (default)
-annexB/language/global-code/if-decl-else-decl-b-global-skip-early-err-switch.js (default)
-annexB/language/global-code/if-decl-else-decl-b-global-skip-early-err-try.js (default)
-annexB/language/global-code/if-decl-else-stmt-global-no-skip-try.js (default)
-annexB/language/global-code/if-decl-else-stmt-global-skip-early-err-block.js (default)
-annexB/language/global-code/if-decl-else-stmt-global-skip-early-err.js (default)
-annexB/language/global-code/if-decl-else-stmt-global-skip-early-err-switch.js (default)
-annexB/language/global-code/if-decl-else-stmt-global-skip-early-err-try.js (default)
-annexB/language/global-code/if-decl-no-else-global-no-skip-try.js (default)
-annexB/language/global-code/if-decl-no-else-global-skip-early-err-block.js (default)
-annexB/language/global-code/if-decl-no-else-global-skip-early-err.js (default)
-annexB/language/global-code/if-decl-no-else-global-skip-early-err-switch.js (default)
-annexB/language/global-code/if-decl-no-else-global-skip-early-err-try.js (default)
-annexB/language/global-code/if-stmt-else-decl-global-no-skip-try.js (default)
-annexB/language/global-code/if-stmt-else-decl-global-skip-early-err-block.js (default)
-annexB/language/global-code/if-stmt-else-decl-global-skip-early-err.js (default)
-annexB/language/global-code/if-stmt-else-decl-global-skip-early-err-switch.js (default)
-annexB/language/global-code/if-stmt-else-decl-global-skip-early-err-try.js (default)
-annexB/language/global-code/switch-case-global-no-skip-try.js (default)
-annexB/language/global-code/switch-case-global-skip-early-err-block.js (default)
-annexB/language/global-code/switch-case-global-skip-early-err.js (default)
-annexB/language/global-code/switch-case-global-skip-early-err-switch.js (default)
-annexB/language/global-code/switch-case-global-skip-early-err-try.js (default)
-annexB/language/global-code/switch-dflt-global-no-skip-try.js (default)
-annexB/language/global-code/switch-dflt-global-skip-early-err-block.js (default)
-annexB/language/global-code/switch-dflt-global-skip-early-err.js (default)
-annexB/language/global-code/switch-dflt-global-skip-early-err-switch.js (default)
-annexB/language/global-code/switch-dflt-global-skip-early-err-try.js (default)
-annexB/language/statements/try/catch-redeclared-for-in-var.js (default)
-annexB/language/statements/try/catch-redeclared-for-in-var.js (strict mode)
-annexB/language/statements/try/catch-redeclared-for-var.js (default)
-annexB/language/statements/try/catch-redeclared-for-var.js (strict mode)
-annexB/language/statements/try/catch-redeclared-var-statement-captured.js (default)
-annexB/language/statements/try/catch-redeclared-var-statement-captured.js (strict mode)
-annexB/language/statements/try/catch-redeclared-var-statement.js (default)
-annexB/language/statements/try/catch-redeclared-var-statement.js (strict mode)
 language/block-scope/syntax/redeclaration/async-function-declaration-attempt-to-redeclare-with-async-function-declaration.js (default)
 language/block-scope/syntax/redeclaration/async-function-declaration-attempt-to-redeclare-with-async-function-declaration.js (strict mode)
 language/block-scope/syntax/redeclaration/async-function-declaration-attempt-to-redeclare-with-class-declaration.js (default)
@@ -261,7 +173,6 @@ language/statements/class/syntax/early-errors/class-definition-evaluation-block-
 language/statements/class/syntax/early-errors/class-definition-evaluation-scriptbody-duplicate-binding.js (default)
 language/statements/class/syntax/early-errors/class-definition-evaluation-scriptbody-duplicate-binding.js (strict mode)
 language/statements/const/syntax/const-declaring-let-split-across-two-lines.js (default)
-language/statements/do-while/labelled-fn-stmt.js (default)
 language/statements/for/head-let-bound-names-in-stmt.js (default)
 language/statements/for/head-let-bound-names-in-stmt.js (strict mode)
 language/statements/for-in/head-const-bound-names-in-stmt.js (default)
@@ -270,15 +181,8 @@ language/statements/for-in/head-const-bound-names-let.js (default)
 language/statements/for-in/head-let-bound-names-in-stmt.js (default)
 language/statements/for-in/head-let-bound-names-in-stmt.js (strict mode)
 language/statements/for-in/head-let-bound-names-let.js (default)
-language/statements/for-in/labelled-fn-stmt-const.js (default)
-language/statements/for-in/labelled-fn-stmt-let.js (default)
-language/statements/for-in/labelled-fn-stmt-lhs.js (default)
-language/statements/for-in/labelled-fn-stmt-var.js (default)
 language/statements/for-in/let-block-with-newline.js (default)
 language/statements/for-in/let-identifier-with-newline.js (default)
-language/statements/for/labelled-fn-stmt-expr.js (default)
-language/statements/for/labelled-fn-stmt-let.js (default)
-language/statements/for/labelled-fn-stmt-var.js (default)
 language/statements/for/let-block-with-newline.js (default)
 language/statements/for/let-identifier-with-newline.js (default)
 language/statements/for-of/head-const-bound-names-in-stmt.js (default)
@@ -287,10 +191,6 @@ language/statements/for-of/head-const-bound-names-let.js (default)
 language/statements/for-of/head-let-bound-names-in-stmt.js (default)
 language/statements/for-of/head-let-bound-names-in-stmt.js (strict mode)
 language/statements/for-of/head-let-bound-names-let.js (default)
-language/statements/for-of/labelled-fn-stmt-const.js (default)
-language/statements/for-of/labelled-fn-stmt-let.js (default)
-language/statements/for-of/labelled-fn-stmt-lhs.js (default)
-language/statements/for-of/labelled-fn-stmt-var.js (default)
 language/statements/for-of/let-block-with-newline.js (default)
 language/statements/for-of/let-identifier-with-newline.js (default)
 language/statements/for-await-of/let-block-with-newline.js (default)
@@ -394,11 +294,11 @@ language/statements/switch/syntax/redeclaration/var-declaration-attempt-to-redec
 language/statements/switch/syntax/redeclaration/var-declaration-attempt-to-redeclare-with-function-declaration.js (strict mode)
 language/statements/switch/syntax/redeclaration/var-declaration-attempt-to-redeclare-with-generator-declaration.js (default)
 language/statements/switch/syntax/redeclaration/var-declaration-attempt-to-redeclare-with-generator-declaration.js (strict mode)
-language/statements/while/labelled-fn-stmt.js (default)
 language/statements/while/let-block-with-newline.js (default)
 language/statements/while/let-identifier-with-newline.js (default)
-language/statements/with/labelled-fn-stmt.js (default)
 language/statements/with/let-block-with-newline.js (default)
 language/statements/with/let-identifier-with-newline.js (default)
+language/statements/try/early-catch-var.js (default)
+language/statements/try/early-catch-var.js (strict mode)
 language/white-space/mongolian-vowel-separator.js (default)
 language/white-space/mongolian-vowel-separator.js (strict mode)

src/expression.js

@@ -20,7 +20,7 @@ import {types as tt} from "./tokentype"
 import {Parser} from "./state"
 import {DestructuringErrors} from "./parseutil"
 import {lineBreak} from "./whitespace"
-import {SCOPE_FUNCTION, SCOPE_ARROW, SCOPE_ASYNC, SCOPE_GENERATOR} from "./scopeflags"
+import {SCOPE_FUNCTION, SCOPE_ARROW, SCOPE_ASYNC, SCOPE_GENERATOR, BIND_OUTSIDE, BIND_VAR} from "./scopeflags"
 
 const pp = Parser.prototype
 
@@ -323,7 +323,7 @@ pp.parseExprAtom = function(refDestructuringErrors) {
     let startPos = this.start, startLoc = this.startLoc, containsEsc = this.containsEsc
     let id = this.parseIdent(this.type !== tt.name)
     if (this.options.ecmaVersion >= 8 && !containsEsc && id.name === "async" && !this.canInsertSemicolon() && this.eat(tt._function))
-      return this.parseFunction(this.startNodeAt(startPos, startLoc), false, false, true)
+      return this.parseFunction(this.startNodeAt(startPos, startLoc), 0, false, true)
     if (canBeArrow && !this.canInsertSemicolon()) {
       if (this.eat(tt.arrow))
         return this.parseArrowExpression(this.startNodeAt(startPos, startLoc), [id], false)
@@ -374,7 +374,7 @@ pp.parseExprAtom = function(refDestructuringErrors) {
   case tt._function:
     node = this.startNode()
     this.next()
-    return this.parseFunction(node, false)
+    return this.parseFunction(node, 0)
 
   case tt._class:
     return this.parseClass(this.startNode(), false)
@@ -768,10 +768,8 @@ pp.parseFunctionBody = function(node, isArrowFunction) {
   }
   this.exitScope()
 
-  if (this.strict && node.id) {
-    // Ensure the function name isn't a forbidden identifier in strict mode, e.g. 'eval'
-    this.checkLVal(node.id, "none")
-  }
+  // Ensure the function name isn't a forbidden identifier in strict mode, e.g. 'eval'
+  if (this.strict && node.id) this.checkLVal(node.id, BIND_OUTSIDE)
   this.strict = oldStrict
 }
 
@@ -787,7 +785,7 @@ pp.isSimpleParamList = function(params) {
 pp.checkParams = function(node, allowDuplicates) {
   let nameHash = {}
   for (let param of node.params)
-    this.checkLVal(param, "var", allowDuplicates ? null : nameHash)
+    this.checkLVal(param, BIND_VAR, allowDuplicates ? null : nameHash)
 }
 
 // Parses a comma-separated list of expressions, and returns them as

src/lval.js

@@ -1,6 +1,7 @@
 import {types as tt} from "./tokentype"
 import {Parser} from "./state"
 import {has} from "./util"
+import {BIND_NONE, BIND_OUTSIDE} from "./scopeflags"
 
 const pp = Parser.prototype
 
@@ -185,7 +186,7 @@ pp.parseMaybeDefault = function(startPos, startLoc, left) {
 // 'let' indicating that the lval creates a lexical ('let' or 'const') binding
 // 'none' indicating that the binding should be checked for illegal identifiers, but not for duplicate references
 
-pp.checkLVal = function(expr, bindingType, checkClashes) {
+pp.checkLVal = function(expr, bindingType = BIND_NONE, checkClashes) {
   switch (expr.type) {
   case "Identifier":
     if (this.strict && this.reservedWordsStrictBind.test(expr.name))
@@ -195,12 +196,7 @@ pp.checkLVal = function(expr, bindingType, checkClashes) {
         this.raiseRecoverable(expr.start, "Argument name clash")
       checkClashes[expr.name] = true
     }
-    if (bindingType && bindingType !== "none") {
-      if (bindingType === "var" && !this.canDeclareVarName(expr.name) ||
-          bindingType !== "var" && !this.canDeclareLexicalName(expr.name))
-        this.raiseRecoverable(expr.start, `Identifier '${expr.name}' has already been declared`)
-      this.declareName(expr.name, bindingType === "var")
-    }
+    if (bindingType !== BIND_NONE && bindingType !== BIND_OUTSIDE) this.declareName(expr.name, bindingType, expr.start)
     break
 
   case "MemberExpression":

src/scope.js

@@ -1,5 +1,5 @@
 import {Parser} from "./state"
-import {SCOPE_FUNCTION} from "./scopeflags"
+import {SCOPE_FUNCTION, SCOPE_SIMPLE_CATCH, BIND_LEXICAL, BIND_SIMPLE_CATCH, BIND_FUNCTION} from "./scopeflags"
 
 const pp = Parser.prototype
 
@@ -23,39 +23,28 @@ pp.exitScope = function() {
   this.scopeStack.pop()
 }
 
-/**
- * A name can be declared with `var` if there are no variables with the same name declared with `let`/`const`
- * in the current lexical scope or any of the parent lexical scopes in this function.
- */
-pp.canDeclareVarName = function(name) {
-  for (let i = this.scopeStack.length - 1; i >= 0; --i) {
-    const currentScope = this.scopeStack[i]
-    if (currentScope.lexical.indexOf(name) !== -1) return false
-    if (currentScope.flags & SCOPE_FUNCTION) return true
-  }
-  return true
-}
-
-/**
- * A name can be declared with `let`/`const` if there are no variables with the same name declared with `let`/`const`
- * in the current scope, and there are no variables with the same name declared with `var` in the current scope or in
- * any child lexical scopes in this function.
- */
-pp.canDeclareLexicalName = function(name) {
-  const currentScope = this.currentScope()
-  return currentScope.lexical.indexOf(name) === -1 && currentScope.var.indexOf(name) === -1
-}
-
-pp.declareName = function(name, isVar) {
-  if (isVar) {
-    for (let i = this.scopeStack.length - 1; i >= 0; i--) {
-      let scope = this.scopeStack[i]
+pp.declareName = function(name, bindingType, pos) {
+  let redeclared = false
+  if (bindingType === BIND_LEXICAL) {
+    const scope = this.currentScope()
+    redeclared = scope.lexical.indexOf(name) > -1 || scope.var.indexOf(name) > -1
+    scope.lexical.push(name)
+  } else if (bindingType === BIND_SIMPLE_CATCH) {
+    const scope = this.currentScope()
+    scope.lexical.push(name)
+  } else if (bindingType === BIND_FUNCTION) {
+    const scope = this.currentScope()
+    redeclared = scope.lexical.indexOf(name) > -1
+    scope.var.push(name)
+  } else {
+    for (let i = this.scopeStack.length - 1; i >= 0; --i) {
+      const scope = this.scopeStack[i]
+      if (scope.lexical.indexOf(name) > -1 && !(scope.flags & SCOPE_SIMPLE_CATCH) && scope.lexical[0] === name) redeclared = true
       scope.var.push(name)
       if (scope.flags & SCOPE_FUNCTION) break
     }
-  } else {
-    this.currentScope().lexical.push(name)
   }
+  if (redeclared) this.raiseRecoverable(pos, `Identifier '${name}' has already been declared`)
 }
 
 pp.currentScope = function() {

src/scopeflags.js

@@ -1 +1,3 @@
-export const SCOPE_FUNCTION = 1, SCOPE_ASYNC = 2, SCOPE_GENERATOR = 4, SCOPE_ARROW = 8, SCOPE_CATCH = 16
+export const SCOPE_FUNCTION = 1, SCOPE_ASYNC = 2, SCOPE_GENERATOR = 4, SCOPE_ARROW = 8, SCOPE_SIMPLE_CATCH = 16
+
+export const BIND_NONE = 0, BIND_VAR = 1, BIND_LEXICAL = 2, BIND_FUNCTION = 3, BIND_SIMPLE_CATCH = 4, BIND_OUTSIDE = 5

src/state.js

@@ -115,7 +115,7 @@ export class Parser {
     return this.parseTopLevel(node)
   }
 
-  get inFunction() { return this.currentFunctionScope() != this.scopeStack[0] }
+  get inFunction() { return this.currentFunctionScope() !== this.scopeStack[0] }
   get inGenerator() { return (this.currentFunctionScope().flags & SCOPE_GENERATOR) > 0 }
   get inAsync() { return (this.currentFunctionScope().flags & SCOPE_ASYNC) > 0 }
 }