close
Skip to content

Editorial: stop using strings for special values#2155

Closed
ljharb wants to merge 3 commits into
tc39:mainfrom
ljharb:enumb
Closed

Editorial: stop using strings for special values#2155
ljharb wants to merge 3 commits into
tc39:mainfrom
ljharb:enumb

Conversation

@ljharb
Copy link
Copy Markdown
Member

@ljharb ljharb commented Aug 26, 2020

This was extracted from #2154 so it could land separately.

@ljharb ljharb requested review from a team, bakkot, bmeck, michaelficarra and syg August 26, 2020 23:21
ljharb pushed a commit to ljharb/ecma262 that referenced this pull request Aug 26, 2020
This was extracted from tc39#2154 so it could land separately.
Comment thread spec.html Outdated
ljharb pushed a commit to ljharb/ecma262 that referenced this pull request Aug 26, 2020
This was extracted from tc39#2154 so it could land separately.
ljharb pushed a commit to ljharb/ecma262 that referenced this pull request Aug 26, 2020
This was extracted from tc39#2154 so it could land separately.
Comment thread spec.html Outdated
ljharb added a commit to ljharb/ecma262 that referenced this pull request Aug 26, 2020
This was extracted from tc39#2154 so it could land separately.

Co-authored-by: Jordan Harband <ljharb@gmail.com>
@ljharb ljharb requested a review from devsnek August 26, 2020 23:40
Comment thread spec.html Outdated
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 2, 2020
This was extracted from tc39#2154 so it could land separately.

Co-authored-by: Jordan Harband <ljharb@gmail.com>
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 2, 2020
This was extracted from tc39#2154 so it could land separately.

Co-authored-by: Jordan Harband <ljharb@gmail.com>
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 2, 2020
This was extracted from tc39#2154 so it could land separately.

Co-authored-by: Jordan Harband <ljharb@gmail.com>
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 2, 2020
This was extracted from tc39#2154 so it could land separately.

Co-authored-by: Jordan Harband <ljharb@gmail.com>
@ljharb ljharb requested a review from a team September 2, 2020 23:42
@ljharb ljharb self-assigned this Sep 2, 2020
@bakkot
Copy link
Copy Markdown
Member

bakkot commented Sep 8, 2020

There's another special string: "*", used in ImportEntry Records to indicate that the import request is for the target module's namespace object.

Comment thread spec.html Outdated
1. For each ExportEntry Record _e_ of _module_.[[IndirectExportEntries]], do
1. Let _resolution_ be ? _module_.ResolveExport(_e_.[[ExportName]]).
1. If _resolution_ is *null* or *"ambiguous"*, throw a *SyntaxError* exception.
1. If _resolution_ is *null* or ~ambiguous~ throw a *SyntaxError* exception.
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.

Suggested change
1. If _resolution_ is *null* or ~ambiguous~ throw a *SyntaxError* exception.
1. If _resolution_ is *null* or ~ambiguous~, throw a *SyntaxError* exception.

Comment thread spec.html
1. Assert: Type(_name_) is String.
1. Assert: Either Type(_name_) is String, or _name_ is ~default~.
1. If _environment_ is not *undefined*, then
1. Perform _environment_.InitializeBinding(_name_, _value_).
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.

I am pretty sure we shouldn't actually be attempting to initialize a binding named ~default~.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed in 870e5f7

@ljharb
Copy link
Copy Markdown
Member Author

ljharb commented Sep 8, 2020

Handled the ImportEntry Records string in 158950a.

Comment thread spec.html
1. Let _value_ be ? BindingClassDeclarationEvaluation of |ClassDeclaration|.
1. Let _className_ be the sole element of BoundNames of |ClassDeclaration|.
1. If _className_ is *"\*default\*"*, then
1. If _className_ is ~default~, then
Copy link
Copy Markdown
Member

@bakkot bakkot Sep 8, 2020

Choose a reason for hiding this comment

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

I don't actually understand why we're making/initializing a binding here at all. Possibly some interaction with module instantiation I'm missing? Anyone know what's up with that?

Comment thread spec.html
<emu-grammar>GeneratorDeclaration : `function` `*` `(` FormalParameters `)` `{` GeneratorBody `}`</emu-grammar>
<emu-alg>
1. Return &laquo; *"\*default\*"* &raquo;.
1. Return &laquo; ~default~ &raquo;.
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.

I believe this still ends up making a binding named ~default~ in InitializeEnvironment of Source Text Module Records, step 23.

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 10, 2021
…tcampbell

The "Arbitrary module namespace identifier names" spec PR allows to use "*" as
a module export name, so we can no longer use that specific string to denote
star-imports/exports. Probably the easiest way to work around this new
restriction is to replace "*" with a nullptr string.

Spec change: tc39/ecma262#2155

Differential Revision: https://phabricator.services.mozilla.com/D101013
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Feb 11, 2021
…tcampbell

The "Arbitrary module namespace identifier names" spec PR allows to use "*" as
a module export name, so we can no longer use that specific string to denote
star-imports/exports. Probably the easiest way to work around this new
restriction is to replace "*" with a nullptr string.

Spec change: tc39/ecma262#2155

Differential Revision: https://phabricator.services.mozilla.com/D101013
@ljharb ljharb force-pushed the master branch 3 times, most recently from 3d0c24c to 7a79833 Compare June 29, 2021 02:21
Comment thread spec.html
</td>
<td>
The name under which the desired binding is exported by the module identified by [[ModuleRequest]]. *null* if the |ExportDeclaration| does not have a |ModuleSpecifier|. *"\*"* indicates that the export request is for all exported bindings.
The name under which the desired binding is exported by the module identified by [[ModuleRequest]]. *null* if the |ExportDeclaration| does not have a |ModuleSpecifier|. ~*~ indicates that the export request is for all exported bindings.
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.

I would prefer to use something like ~all~ for this.

@syg syg added the editor call to be discussed in the next editor call label Oct 25, 2021
@ljharb ljharb closed this in #2566 Nov 5, 2021
@bakkot bakkot removed the editor call to be discussed in the next editor call label Nov 10, 2021
@ljharb ljharb deleted the enumb branch January 26, 2022 18:31
bhearsum pushed a commit to mozilla-releng/staging-firefox that referenced this pull request May 1, 2025
…tcampbell

The "Arbitrary module namespace identifier names" spec PR allows to use "*" as
a module export name, so we can no longer use that specific string to denote
star-imports/exports. Probably the easiest way to work around this new
restriction is to replace "*" with a nullptr string.

Spec change: tc39/ecma262#2155

Differential Revision: https://phabricator.services.mozilla.com/D101013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants