Correct block-decl-func-skip-arguments for the current FunctionDeclarationInstantiation - #5112
Open
lahma wants to merge 2 commits into
Open
Correct block-decl-func-skip-arguments for the current FunctionDeclarationInstantiation#5112lahma wants to merge 2 commits into
lahma wants to merge 2 commits into
Conversation
…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
approved these changes
Aug 20, 2026
gibson042
left a comment
Member
There was a problem hiding this comment.
Thanks! LGTM, but this is covering some intricate operations and I would like other maintainers to have the opportunity for review as well.
…nDeclarationInstantiation Co-authored-by: Richard Gibson <richard.gibson@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5113
annexB/language/function-code/block-decl-func-skip-arguments.jswas written in 2017 against an edition of FunctionDeclarationInstantiation that appended"arguments"toparameterNames— which its owninfoblock quotes: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
paramBindingsas "the list-concatenation of paramNames and « "arguments" »", while the web-compat eligibility test consultsparamNames:So a block-level function named
argumentsis eligible. Within that branch, thefuncName 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: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.jsasserts the opposite for the identical shape:Implementations
Both engines side with the staging file. On V8 (node 24.19.0):
SpiderMonkey agrees — the staging file is its own test, contributed upstream.
What this PR changes
infoblock is rewritten against the current text, with a note explaining why the eligibility test and the"arguments"guard consult different things.argumentsis inparamNames, 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.