Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

B.3.2.1 - Which FunctionDeclaration parse nodes list does it iterate? #2663

Closed
nicolo-ribaudo opened this issue Feb 13, 2022 · 6 comments · Fixed by #3361
Closed

B.3.2.1 - Which FunctionDeclaration parse nodes list does it iterate? #2663

nicolo-ribaudo opened this issue Feb 13, 2022 · 6 comments · Fixed by #3361

Comments

@nicolo-ribaudo
Copy link
Member

Description:

We are trying to fix the handling of function declarations in blocks in Babel (babel/babel#14203), and I don't understand what the spec does.

At step 29.a of B.3.2.1, it says:

a. For each FunctionDeclaration f that is directly contained in the StatementList of a Block, CaseClause, or DefaultClause, do

However, usually "For each" iterates the elements of a list. Is it iterating over the functions (deeply) contained within the function that is currently being instantiated? Or is it something like "If the current FunctionDeclarationInstantiation call has been caused by evaluating a FunctionDeclaration parse node, let f be that parse node"?

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 13, 2022

I agree that it's odd: the criterion "each FunctionDeclarationthat is directly contained in the StatementList of a Block, CaseClause, or DefaultClause" makes it sound like we need to look at any Block, CaseClause, or DefaultClause, anywhere in the Script.

In ES6, that step is:

For each FunctionDeclaration f in varDeclarations that is directly contained in the StatementList of a Block, CaseClause, or DefaultClause,

(emphasis mine), which makes it clear you don't have to go looking everywhere. But the phrase "in varDeclarations" was removed in efbfc88 to resolve https://tc39.es/archives/bugzilla/4427/, which links back to an issue that @bakkot raised in 2015.

However, it looks like the change that @allenwb suggested in 4427 wasn't applied correctly: in addition to dropping "in varDeclarations", he also appended "Contained within code" (where code is the FunctionBody of the function that FunctionDeclarationInstantiation is being called on), but that wasn't added.

@nicolo-ribaudo
Copy link
Member Author

That might still be somewhat problematic because when instantiating f in

function f() {
  function g() {
    { function h() {} }
  }
}

we don't want to look at h.

@jmdyck
Copy link
Collaborator

jmdyck commented Feb 13, 2022

In the phrase "Contained within code", I suspect the capital 'C' alludes to the 'Contains' SDO, which avoids looking inside nested functionDecls.

(To make it a proper invocation of 'Contains', I think it would need to be something like "... any Block, CaseClause, or DefaultClause x such that code Contains x".)

@bathos
Copy link
Contributor

bathos commented Feb 14, 2022

I read "directly" as specifying that h (indirectly contained) is not considered.

@nicolo-ribaudo
Copy link
Member Author

@bathos In my example the function is directly contained in a block. I'll open a PR with "Contains".

@bathos
Copy link
Contributor

bathos commented Feb 14, 2022

Ah, got it — “directly contained” only describes the declaration relative to the block, not the block relative to the “root” statement list.

jmdyck added a commit to jmdyck/ecma262 that referenced this issue Jun 27, 2024
…tantiation

This PR completes a small bugfix from 9 years ago.

Fixes tc39#2663.

----

History:

2015-07-17:
@bakkot identifies a problem in Annex B's
"Changes to FunctionDeclarationInstantiation":
https://esdiscuss.org/topic/block-level-function-declarations-web-legacy-compatibility-bug

To remedy this, @allenwb submits bug 4427:
https://tc39.es/archives/bugzilla/4427/
in which he recommends changing
> For each FunctionDeclaration _f_ **in _varDeclarations_** that is directly contained in the |StatementList| of a |Block|, |CaseClause|, or |DefaultClause|,

to
> For each FunctionDeclaration _f_ that is directly contained in the |StatementList| of a |Block|, |CaseClause|, or |DefaultClause| **Contained within _code_**,

(emphasis mine).

2015-10-29:
@anba submits PR tc39#141, claiming to fix bug 4427.
It deletes "in _varDeclarations_",
but doesn't add "Contained within _code_".
My guess is, this was just an oversight.

2015-11-02:
PR tc39#141 is merged to master as commit efbfc88.

2022-02-13:
@nicolo-ribaudo raises issue tc39#2663 about this,
and says he'd open a PR to fix it,
but I don't think that happened.

2024-06-26:
@gibson042 raises the problem again, in a commment on PR tc39#2952:
tc39#2952 (comment)
jmdyck added a commit to jmdyck/ecma262 that referenced this issue Jul 11, 2024
…tantiation

This PR completes a small bugfix from 9 years ago.

Fixes tc39#2663.

----

History:

2015-07-17:
@bakkot identifies a problem in Annex B's
"Changes to FunctionDeclarationInstantiation":
https://esdiscuss.org/topic/block-level-function-declarations-web-legacy-compatibility-bug

To remedy this, @allenwb submits bug 4427:
https://tc39.es/archives/bugzilla/4427/
in which he recommends changing
> For each FunctionDeclaration _f_ **in _varDeclarations_** that is directly contained in the |StatementList| of a |Block|, |CaseClause|, or |DefaultClause|,

to
> For each FunctionDeclaration _f_ that is directly contained in the |StatementList| of a |Block|, |CaseClause|, or |DefaultClause| **Contained within _code_**,

(emphasis mine).

2015-10-29:
@anba submits PR tc39#141, claiming to fix bug 4427.
It deletes "in _varDeclarations_",
but doesn't add "Contained within _code_".
My guess is, this was just an oversight.

2015-11-02:
PR tc39#141 is merged to master as commit efbfc88.

2022-02-13:
@nicolo-ribaudo raises issue tc39#2663 about this,
and says he'd open a PR to fix it,
but I don't think that happened.

2024-06-26:
@gibson042 raises the problem again, in a commment on PR tc39#2952:
tc39#2952 (comment)
jmdyck added a commit to jmdyck/ecma262 that referenced this issue Jul 11, 2024
Change the 3 occurrences of
    a |Block|, |CaseClause|, or |DefaultClause| Contained within _code_
to
    any |Block|, |CaseClause|, or |DefaultClause| _x_ such that _code_ Contains _x_ is *true*

(See tc39#2663 (comment))
ljharb pushed a commit to jmdyck/ecma262 that referenced this issue Jul 17, 2024
Change the 3 occurrences of
    a |Block|, |CaseClause|, or |DefaultClause| Contained within _code_
to
    any |Block|, |CaseClause|, or |DefaultClause| _x_ such that _code_ Contains _x_ is *true*

(See tc39#2663 (comment))
@ljharb ljharb closed this as completed in 5885b6c Jul 17, 2024
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 a pull request may close this issue.

3 participants