Skip to content

Correct block-decl-func-skip-arguments for the current FunctionDeclarationInstantiation - #5112

Open
lahma wants to merge 2 commits into
tc39:mainfrom
lahma:fix/block-decl-func-arguments-alias
Open

Correct block-decl-func-skip-arguments for the current FunctionDeclarationInstantiation#5112
lahma wants to merge 2 commits into
tc39:mainfrom
lahma:fix/block-decl-func-arguments-alias

Conversation

@lahma

@lahma lahma commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Fixes #5113

annexB/language/function-code/block-decl-func-skip-arguments.js was written in 2017 against an edition of FunctionDeclarationInstantiation that appended "arguments" to parameterNames — which its own info block quotes:

  1. If argumentsObjectNeeded is true, then
    f. Append "arguments" to parameterNames.

Under that edition a block-level function arguments(){} was ineligible for the Annex B var alias entirely, so the arguments object survived the block. That step no longer exists.

The current algorithm keeps two lists. Step 22.h builds paramBindings as "the list-concatenation of paramNames and « "arguments" »", while the web-compat eligibility test consults paramNames:

If replacing the FunctionDeclaration funcDecl with a VariableStatement that has funcName as a BindingIdentifier would not produce any Early Errors for func and paramNames does not contain funcName, then

So a block-level function named arguments is eligible. Within that branch, the funcName is not "arguments" guard covers only the creation of a new var binding — that binding already exists, holding the arguments object — while the alternative declaration-evaluation step is a sibling step that still runs:

iii. Perform ! funcEnv.SetMutableBinding(funcName, funcObj, false).

The arguments object is therefore replaced once the declaration is evaluated, and the three assertions after the block are wrong.

This file contradicts another test in this suite

staging/sm/lexical-environment/block-scoped-functions-annex-b-arguments.js asserts the opposite for the identical shape:

assert.sameValue(typeof arguments, "object");
{ function arguments() {} }
assert.sameValue(typeof arguments, "function");

Implementations

Both engines side with the staging file. On V8 (node 24.19.0):

(function(){ { function arguments(){} } return typeof arguments })()        // "function"
(function(x){ { function arguments(){} } return typeof arguments })()       // "function"
(function(..._){ { function arguments(){} } return typeof arguments })()    // "function"
(function(arguments){ { function arguments(){} } return typeof arguments })(1)  // "number"

SpiderMonkey agrees — the staging file is its own test, contributed upstream.

What this PR changes

  • The three post-block assertions now expect the function.
  • The info block is rewritten against the current text, with a note explaining why the eligibility test and the "arguments" guard consult different things.
  • Adds a fourth case pinning the other side of the distinction: a formal parameter really named arguments is in paramNames, so the declaration is not eligible and the parameter is left alone. That case had no coverage and is what makes the two lists observably different.

The assertions before and inside the block are unchanged and were already correct. The edited file passes on V8; the filename is left alone to avoid breaking downstream exclusion lists.

Found while enabling staging/ in Jint (sebastienros/jint#3021), where the two files cannot both pass.

…ationInstantiation

The test was written in 2017 against an edition whose FDI appended "arguments"
to parameterNames, which made a block-level `function arguments(){}` ineligible
for the Annex B var alias entirely.

The current algorithm keeps two lists: paramNames, which the eligibility test in
step 35.a.i.2 consults, and paramBindings, which is paramNames plus "arguments"
(step 22.h). A block-level function named 'arguments' is therefore eligible, and
the `funcName is not "arguments"` guard covers only the creation of a new var
binding -- that binding already exists, holding the arguments object. The
assignment performed when the declaration is evaluated still runs, so the
arguments object is replaced.

V8 and SpiderMonkey both behave this way, and this file contradicted
staging/sm/lexical-environment/block-scoped-functions-annex-b-arguments.js,
which asserts the opposite and is in this same suite.

Updates the three assertions after the block, rewrites the info block against
the current text, and adds a case pinning the other side of the distinction: a
formal parameter really named 'arguments' IS in paramNames, so the declaration
is not eligible and the parameter is left alone.

@gibson042 gibson042 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! LGTM, but this is covering some intricate operations and I would like other maintainers to have the opportunity for review as well.

Comment thread test/annexB/language/function-code/block-decl-func-skip-arguments.js Outdated
Comment thread test/annexB/language/function-code/block-decl-func-skip-arguments.js Outdated
Comment thread test/annexB/language/function-code/block-decl-func-skip-arguments.js Outdated
Comment thread test/annexB/language/function-code/block-decl-func-skip-arguments.js Outdated
Comment thread test/annexB/language/function-code/block-decl-func-skip-arguments.js Outdated
Comment thread test/annexB/language/function-code/block-decl-func-skip-arguments.js Outdated
…nDeclarationInstantiation

Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

annexB block-decl-func-skip-arguments contradicts staging/sm block-scoped-functions-annex-b-arguments

2 participants