Ch.12, cumulative bugs & suggestions
[Note: this is against Rev. 23, some things might have been fixed or changed already.]
TECHNICAL ISSUES
12.2.2.1: the actual algorithm seems to be gone amiss
12.2.5.3: there seem to be several cases missing for this algorithm
12.2.5.9, 1st case: call to DefineProperty seems bogus in step 6, already happened at this point
12.2.5.9, 3rd case: shouldn't setting [[MethodName]] be skipped if propKey is a symbol?
12.3.1.1: there seem to be some cases missing for MemberExpression and CallExpression
12.3.3.1: this doesn't handle tail calls, which it should. Why not simply say that "new NewExpression" is evaluated as "new NewExpression()"?
12.3.4.1, 1st case, step 2.a: "check if direct eval" -- is this a todo?
12.5.4.1, 2nd bullet: this formulation doesn't really make sense to me; whatever phrase you can derive via CoverParenthesizedEAAPL will always have outermost parentheses, so the previous rule would never apply to it.
12.5.6.1, step 2.b: wrong level, this should be step 3
12.14.1, 1st case, 1st bullet: this seems to disallow parentheses around LHS array or object patterns, which seem unfortunate. Is that intentional?
12.14.1, 1st case 3rd bullet: "can be statically determined" is far too vague here. Consider "const x = 0; with ({__proto__: null}) x = 1;" -- it is possible (and in this case even fairly easy) to statically determine the condition. Or "const x = 0; { eval(""); x = 1}". But those are clearly not cases that you want to require an error for. The spec needs to be more specific, probably by explicitly ruling out cases with intervening object environments, or scopes with sloppy eval.
12.14.1, 2nd case 1st bullet: dito
12.14.5.1, 3rd case, 3rd bullet: dito
12.14.5.2, 'AssignmentProperty: IdentifierReference Initializer_opt' case, step 4.b: the call to ToObject seems bogus here
12.14.5.3, AssignmentElement case: steps 1-6 as well as 8 need to be substeps of 1.
12.14.5.3, AssignmentRestElement case: this algorithm is not sufficiently recursive; it needs to allow the case where DestructuringAssignmentTarget is itself an object or array literal
..., step 5.d: this should be step 5.c.i
12.14.5.4, step 1: "name" -> "propertyName"
SUGGESTIONS
12.1.1, Identifier :: IdentifierName but not ...: why not combine the first two bullets?
12.2.3: Literal/ValueLiteral nonterminal distinction seems redundant, drop?
12.2.3.1: Perhaps rename CheckObjectCoercible -- the current name suggests a predicate or check that should return either Bool or nothing.
12.2.4.1.2, case for SpreadElement: ...AssignmentExpression, step 4: seems redundant (subsumed by step 5)
12.2.4.1.3, 1st production: perhaps assert that the Put in step 3 can't fail?
12.2.4.1.3, 2nd production: likewise step 4 here
12.2.4.1.3, 3rd production: and step 5 here
12.2.4.2.2: it would seem more modular and extensible to fold the type-check into BindingInitialization itself
12.2.4.2.4: fold steps 3-4 into GetIterator?
12.3.4.2: perhaps fold type check in step 5 into the IsCallable predicate?
12.3.5.2, 2nd case: for consistency with ordinary property access, why not describe the 2nd case via desugaring into the first?
12.3.6.1, 3rd case: perhaps fold type check in step 5 into GetIterator?
12.3.6.1, 4th case, step 6: this seems like a very convoluted way of saying "append arg to list and return the result" :)
12.9.4: perhaps fold type check in step 1 into GetMethod?
TYPOS & FORMATTING
12.1.1, IdentifierReference: Identifier, bullet: formatting after "Identifier"
12.1.2: "This is the case for var statements [+and] formal parameters"
12.1.3, first rule: "IdentifierName" -> "Identifier"
12.1.4: formatting of "StringValue"
12.2.0: weird section numbering?
12.2.2: "See 12.1 for ..." - wrong xref?
12.2.5.1, 1st bullet: formatting of PropertyNameList
12.2.5.6: formatting of "PropName"
12.2.7.2, Note: "In that case [-use] other ... means must be used"
12.3.2, 1st grammar: formatting
12.2.5.3, 12.3.1.1: only cases where the default algorithm in 5.3 doesn't apply, need explicit algorithms. Are there specific productions that you think need to this?
12.2.5.9 #2: no [[MethodName]] can be a symbol.
12.3.4.1: yes
Bug 3148 created for "can be statically determined" items
Re: suggestion 12.9.4. For retty much every other use of GetMethod we already know that the first argument is an object.
I think everything here has been addressed in rev26 or rev27 or are otherwise stale.