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

Editorial: Fix old bug in Annex B's changes to FunctionDeclarationInstantiation #3361

Merged
merged 2 commits into from
Jul 17, 2024

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Jun 27, 2024

This PR completes a small bugfix from 9 years ago.

Fixes #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 #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 #141 is merged to master as commit efbfc88.

2022-02-13:
@nicolo-ribaudo raises issue #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 #2952:
#2952 (comment)

Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

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

lol

@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Jul 10, 2024
spec.html Outdated
@@ -50961,7 +50961,7 @@ <h1>Changes to FunctionDeclarationInstantiation</h1>
<p>During FunctionDeclarationInstantiation the following steps are performed in place of step <emu-xref href="#step-functiondeclarationinstantiation-web-compat-insertion-point"></emu-xref>:</p>
<emu-alg replaces-step="step-functiondeclarationinstantiation-web-compat-insertion-point">
1. If _strict_ is *false*, then
1. For each |FunctionDeclaration| _f_ that is directly contained in the |StatementList| of a |Block|, |CaseClause|, or |DefaultClause|, do
1. For each |FunctionDeclaration| _f_ that is directly contained in the |StatementList| of a |Block|, |CaseClause|, or |DefaultClause| Contained within _code_, do
Copy link
Member

Choose a reason for hiding this comment

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

In the editor call, we decided that we would prefer the form ... any |Block|, |CaseClause|, or |DefaultClause| _x_ such that _code_ Contains _x_ is *true* that you mentioned. We will explore dropping the is *true* from here and in other places in #3370.

We should also update the two existing uses of Contained within in B.3.2.2 and B.3.2.3 to match this phrasing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added a commit to accomplish this.

@michaelficarra michaelficarra removed the editor call to be discussed in the next editor call label Jul 10, 2024
@syg syg added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jul 17, 2024
…tantiation (tc39#3361)

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)
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 merged commit f2b2d52 into tc39:main Jul 17, 2024
7 checks passed
@jmdyck jmdyck deleted the Contained_within_code branch July 26, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change ready to merge Editors believe this PR needs no further reviews, and is ready to land. spec bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

B.3.2.1 - Which FunctionDeclaration parse nodes list does it iterate?
4 participants