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: Ensure Abstract Closures don't re-use outer aliases #3016

Merged
merged 1 commit into from
Feb 27, 2023

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Feb 15, 2023

Resolves issue #3010.


Usually, I rename the inner alias, but in one case, I rename the outer...

  • In AsyncFromSyncIteratorContinuation and Await: change the AC's _value_ to _val_.
  • In Promise.prototype.finally: change both ACs' _promise_ to _prom_.
  • In GeneratorStart and AsyncGeneratorStart: change the AC's _genContext_ and _generator_ by prepending 'ac'.
  • In AsyncBlockStart: change the AC's _asyncContext_ likewise.
  • In Proxy.revocable: change the outer _p_ to _proxy_.

Possible wording for editorial convention:

Every Abstract Closure (AC) is defined within an 'outer' algorithm. Inside the AC, the only outer aliases that are visible are the ones explicitly captured in the AC's header. Any other outer aliases cannot be referenced within the AC. Since these aliases are inaccessible, it might seem that their names could be re-used within the AC (e.g., for a parameter or let-alias), but they should not be re-used, for the sake of clarity.

or:

The body of an Abstract Closure (AC) must not reference aliases of the outer algorithm, other than those explicitly captured in the AC's header. Moreover, for clarity, the AC should not re-use the names of outer aliases for its own purposes (even though those aliases are inacessible).

@jmdyck jmdyck marked this pull request as ready for review February 16, 2023 04:32
@michaelficarra michaelficarra added the editor call to be discussed in the next editor call label Feb 21, 2023
@bakkot bakkot removed the editor call to be discussed in the next editor call label Feb 22, 2023
Copy link
Contributor

@bakkot bakkot left a comment

Choose a reason for hiding this comment

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

lgtm pending renames; I don't have strong feelings about the names. Michael wants to avoid prom style stuff, but both single letter and ac-prefixed names are fine.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Feb 26, 2023

Michael wants to avoid prom style stuff, but both single letter and ac-prefixed names are fine.

Done.

@michaelficarra michaelficarra added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change establishes editorial conventions ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants