Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Add OptionalChain support #301

Merged
merged 1 commit into from
Apr 14, 2020
Merged

Add OptionalChain support #301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
out/index.html
node_modules/
47 changes: 46 additions & 1 deletion spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,18 @@ <h1>Updated Syntactic Grammar Productions</h1>
CallExpression[?Yield, ?Await] `.` IdentifierName
CallExpression[?Yield, ?Await] TemplateLiteral[?Yield, ?Await]
<ins>CallExpression[?Yield, ?Await] `.` PrivateIdentifier</ins>

OptionalChain[Yield, Await] :
`?.` `[` Expression[+In, ?Yield, ?Await] `]`
`?.` IdentifierName
`?.` Arguments[?Yield, ?Await]
`?.` TemplateLiteral[?Yield, ?Await, +Tagged]
<ins>`?.` PrivateIdentifier</ins>
Copy link
Member

Choose a reason for hiding this comment

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

Wasn't the idea that this production would be omitted?

Copy link
Member

Choose a reason for hiding this comment

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

See line 198, it’s an explicit syntax error (like the optional template literal syntax, eg)

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. I guess I find this editorial strategy strange--isn't just about everything that's not in the grammar reserved in the future? We haven't used this editorial strategy for other things that we've been thinking about including in the future, like private fields in object literals, or concise private field references (without this). I don't know why this should be treated differently.

Copy link
Member Author

Choose a reason for hiding this comment

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

I left it this way because the only 3 implementations that support privates and optional chain didn't realize that mixing them isn't in the spec. Conveniently none of them screwed up this production, but maybe the next implementation would.

Copy link
Member

Choose a reason for hiding this comment

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

Good point; we should be clear here. How about a note explaining that this production is specifically omitted?

Copy link
Member

Choose a reason for hiding this comment

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

Right, I'm suggesting to remove the grammatical production and early error, and instead simply have a note that explains that the grammatical production is deliberately excluded.

Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI, there are Test262 cases for this tc39/test262#2408. We also considered this issue while implementing private features on WebKit. Unfortunately, I don't remember who report this issue.

Copy link
Member

Choose a reason for hiding this comment

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

@caiolima Are there any tests for the other case, x?.y.#z? (I think we should land this patch, which permits this grammar, but currently, it's not permitted in the grammar.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

@caiolima caiolima Mar 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@caiolima Are there any tests for the other case, x?.y.#z? (I think we should land this patch, which permits this grammar, but currently, it's not permitted in the grammar.)

I don't think we have, but I'll double check and open a PR to add it.

OptionalChain[?Yield, ?Await] `[` Expression[+In, ?Yield, ?Await] `]`
OptionalChain[?Yield, ?Await] `.` IdentifierName
OptionalChain[?Yield, ?Await] Arguments[?Yield, ?Await]
OptionalChain[?Yield, ?Await] TemplateLiteral[?Yield, ?Await, +Tagged]
<ins>OptionalChain[?Yield, ?Await] `.` PrivateIdentifier</ins>
</emu-grammar>
</emu-clause>

Expand Down Expand Up @@ -183,7 +195,7 @@ <h1>Static Semantics: Early Errors</h1>
<emu-grammar>UnaryExpression : `delete` UnaryExpression</emu-grammar>
<ul>
<li>
It is a Syntax Error if the |UnaryExpression| is contained in strict mode code and the derived |UnaryExpression| is <emu-grammar>PrimaryExpression : IdentifierReference</emu-grammar>, <ins><emu-grammar>MemberExpression : MemberExpression `.` PrivateIdentifier</emu-grammar>, or <emu-grammar>CallExpression : CallExpression `.` PrivateIdentifier</emu-grammar></ins>.
It is a Syntax Error if the |UnaryExpression| is contained in strict mode code and the derived |UnaryExpression| is <emu-grammar>PrimaryExpression : IdentifierReference</emu-grammar>, <ins><emu-grammar>MemberExpression : MemberExpression `.` PrivateIdentifier</emu-grammar>, <emu-grammar>CallExpression : CallExpression `.` PrivateIdentifier</emu-grammar>, <emu-grammar>OptionalChain : `?.` PrivateIdentifier</emu-grammar>, or <emu-grammar>OptionalChain : OptionalChain `.` PrivateIdentifier</emu-grammar></ins>.
</li>
<li>
<p>It is a Syntax Error if the derived |UnaryExpression| is
Expand Down Expand Up @@ -224,6 +236,18 @@ <h1>Static Semantics: AllPrivateIdentifiersValid</h1>
1. Return *false*.
</emu-alg>

<emu-grammar>OptionalChain[Yield, Await] : `?.` PrivateIdentifier</emu-grammar>
<emu-alg>
1. If StringValue of |PrivateIdentifier| is in _names_, return *true*.
1. Return *false*.
</emu-alg>

<emu-grammar>OptionalChain[Yield, Await] : OptionalChain[?Yield, ?Await] `.` PrivateIdentifier</emu-grammar>
<emu-alg>
1. If StringValue of |PrivateIdentifier| is in _names_, return *true*.
1. Return *false*.
</emu-alg>

<emu-grammar> ClassBody[Yield, Await] : ClassElementList[?Yield, ?Await] </emu-grammar>
<emu-alg>
1. Let _newNames_ be the concatenation of _names_ with PrivateBoundIdentifiers of |ClassBody|.
Expand Down Expand Up @@ -1544,6 +1568,27 @@ <h1>Runtime Semantics: Evaluation</h1>
</emu-alg>
</emu-clause>

<emu-clause id="sec-private-references-runtime-semantics-chainevaluation">
<h1>Runtime Semantics: ChainEvaluation</h1>
<p>With parameters _baseValue_ and _baseReference_.</p>
<emu-grammar>OptionalChain : `?.` PrivateIdentifier</emu-grammar>
<emu-alg>
1. Let _bv_ be ? RequireObjectCoercible(_baseValue_).
1. Let _fieldNameString_ be the StringValue of |PrivateIdentifier|.
1. Return MakePrivateReference(_bv_, _fieldNameString_).
</emu-alg>

<emu-grammar>OptionalChain : OptionalChain `.` PrivateIdentifier</emu-grammar>
<emu-alg>
1. Let _optionalChain_ be |OptionalChain|.
1. Let _newReference_ be ? ChainEvaluation of _optionalChain_ with arguments _baseValue_ and _baseReference_.
1. Let _newValue_ be ? GetValue(_newReference_).
1. Let _nv_ be ? RequireObjectCoercible(_newValue_).
1. Let _fieldNameString_ be the StringValue of |PrivateIdentifier|.
1. Return MakePrivateReference(_nv_, _fieldNameString_).
</emu-alg>
</emu-clause>

<emu-clause id="sec-reference-specification-type">
<h1>The Reference Specification Type</h1>
<emu-note>
Expand Down