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.

Sign up for GitHub

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

Initial cut at an NNBD specification #293

Merged
merged 15 commits into from
Jun 4, 2019
Merged

Initial cut at an NNBD specification #293

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d8f44d0
Add initial proposal for NNBD feature spec
leafpetersen Mar 29, 2019
f7c278f
Updates
leafpetersen Mar 29, 2019
79e8f58
Address some comments
leafpetersen Apr 17, 2019
a9e7eab
Merge remote-tracking branch 'origin/master' into nnbd
leafpetersen Apr 17, 2019
eec13c1
Proposed grammar for late and required
leafpetersen Apr 18, 2019
1260b58
More grammar changes
leafpetersen Apr 19, 2019
03cc142
Fix function types
leafpetersen Apr 19, 2019
03813d3
Add specification for null-shorting
leafpetersen Apr 23, 2019
5c024e5
Merge remote-tracking branch 'origin/master' into nnbd
leafpetersen May 29, 2019
2466591
Fixes for late
leafpetersen May 29, 2019
186d675
Merge remote-tracking branch 'origin/master' into nnbd
leafpetersen May 31, 2019
b5d301e
Address more comments
leafpetersen Jun 1, 2019
47016c5
Address additional comments
leafpetersen Jun 1, 2019
e721716
Tweaks
leafpetersen Jun 3, 2019
c2b4c0f
Fix wording
leafpetersen Jun 4, 2019
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
2 changes: 2 additions & 0 deletions specification/dart.sty
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
\def\MIXIN{\builtinId{mixin}}
\def\OPERATOR{\builtinId{operator}}
\def\PART{\builtinId{part}}
\def\REQUIRED{\builtinId{required}}
\def\SET{\builtinId{set}}
\def\STATIC{\builtinId{static}}
\def\TYPEDEF{\builtinId{typedef}}
Expand All @@ -45,6 +46,7 @@
\def\IF{\keyword{if}}
\def\IN{\keyword{in}}
\def\IS{\keyword{is}}
\def\LATE{\keyword{late}}
Copy link
Member

Choose a reason for hiding this comment

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

I think late will be fine as a built-in identifier. After all static works as a built-in identifier.

Copy link
Member Author

Choose a reason for hiding this comment

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

@bwilkerson was advocating for this to be a keyword on the grounds that it makes error recovery much more robust. I don't have strong feelings about this - I see arguments on both sides. Taking away variable names from programmers is painful for them (and for us), but making error recovery less robust is a tax on everything.

\def\NEW{\keyword{new}}
\def\NULL{\keyword{null}}
\def\OF{\keyword{of}}
Expand Down
56 changes: 33 additions & 23 deletions specification/dartLangSpec.tex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -832,9 +832,9 @@ \section{Variables}

<declaredIdentifier> ::= <metadata> \COVARIANT{}? <finalConstVarOrType> <identifier>

<finalConstVarOrType> ::= \FINAL{} <type>?
<finalConstVarOrType> ::= \alt \LATE{}? \FINAL{} <type>?
\alt \CONST{} <type>?
\alt <varOrType>
\alt \LATE{}? <varOrType>

<varOrType> ::= \VAR{}
\alt <type>
Expand Down Expand Up @@ -1411,13 +1411,13 @@ \subsection{Formal Parameters}
\begin{grammar}
<formalParameterList> ::= `(' `)'
\alt `(' <normalFormalParameters> `,'? `)'
\alt `(' <normalFormalParameters> `,' <optionalFormalParameters> `)'
\alt `(' <optionalFormalParameters> `)'
\alt `(' <normalFormalParameters> `,' <optionalOrNamedFormalParameters> `)'
\alt `(' <optionalOrNamedFormalParameters> `)'

<normalFormalParameters> ::= \gnewline{}
<normalFormalParameter> (`,' <normalFormalParameter>)*

<optionalFormalParameters> ::= <optionalPositionalFormalParameters>
<optionalOrNamedFormalParameters> ::= <optionalPositionalFormalParameters>
Copy link
Member

Choose a reason for hiding this comment

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

We currently disallow having both optional positional and named parameters.
Will we want to allow optional positional parameters and required named parameters on the same function?

Copy link
Member

Choose a reason for hiding this comment

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

Personally, I'd like to see us allow all four varieties. Whether that happens sooner or later is a separate question, but it makes it much easier to migrate users to a new API if named and optional positional can be mixed, so I'd prefer sooner.

Copy link
Member

Choose a reason for hiding this comment

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

It is top of my list for improvements going on 6.5 years now (https://github.com/dart-lang/sdk/issues/7056, sadly only the nr 27 most wanted feature based on 👍 count).

Copy link
Member Author

Choose a reason for hiding this comment

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

Will we want to allow optional positional parameters and required named parameters on the same function?

This seems to me raise the same kind of calling convention questions that other combinations raise.

class A {
  void foo({required int x}) {}
}

class B extends A {
  void foo({int x}) {}
}

class C extends A {
  void foo([int y], {required int x}) {}
}

A call site that looks like f(A x) => x.foo(x : 3) doesn't know whether the receiver expects an optional named parameter, or a required named parameter, or a required named parameter and one or more optional parameters.

I'm not entirely unsympathetic to broadening our calling conventions, but I think that's a separate change that we need to make with full buy in from the various teams.

\alt <namedFormalParameters>

<optionalPositionalFormalParameters> ::= \gnewline{}
Expand Down Expand Up @@ -1485,8 +1485,8 @@ \subsubsection{Optional Formals}
\begin{grammar}
<defaultFormalParameter> ::= <normalFormalParameter> (`=' <expression>)?

<defaultNamedParameter> ::= <normalFormalParameter> (`=' <expression>)?
\alt <normalFormalParameter> ( `:' <expression>)?
<defaultNamedParameter> ::= \REQUIRED{}? <normalFormalParameter> (`=' <expression>)?
\alt \REQUIRED{}? <normalFormalParameter> ( `:' <expression>)?
leafpetersen marked this conversation as resolved.
Show resolved Hide resolved
\end{grammar}

The form \syntax{<normalFormalParameter> `:' <expression>}
Expand Down Expand Up @@ -1853,9 +1853,12 @@ \section{Classes}
\alt (\EXTERNAL{} \STATIC{}?)? <setterSignature>
\alt \EXTERNAL{}? <operatorSignature>
\alt (\EXTERNAL{} \STATIC{}?)? <functionSignature>
\alt \STATIC{} (\FINAL{} | \CONST{}) <type>? <staticFinalDeclarationList>
\alt \FINAL{} <type>? <initializedIdentifierList>
\alt (\STATIC{} | \COVARIANT{})? (\VAR{} | <type>) <initializedIdentifierList>
\alt \STATIC{} \LATE{}? \FINAL{} <type>? <staticFinalDeclarationList>
\alt \STATIC{} \LATE{}? (\VAR{} | <type>) <initializedIdentifierList>
\alt \STATIC{} \CONST{) <type>? <staticFinalDeclarationList>
\alt \COVARIANT{} \LATE{}? (\VAR{} | <type>) <initializedIdentifierList>
\alt \LATE{}? \FINAL{} <type>? <initializedIdentifierList>
\alt \LATE{}? (\VAR{} | <type>) <initializedIdentifierList>

<staticFinalDeclarationList> ::= \gnewline{}
<staticFinalDeclaration> (`,' <staticFinalDeclaration>)*
Expand Down Expand Up @@ -3007,7 +3010,7 @@ \subsubsection{Generative Constructors}
\alt <assertion>

<fieldInitializer> ::= \gnewline{}
(\THIS{} `.')? <identifier> `=' <conditionalExpression> <cascadeSection>*
(\THIS{} `.')? <identifier> `=' <conditionalExpression> <cascadeSequence>?
\end{grammar}

\LMHash{}%
Expand Down Expand Up @@ -5888,7 +5891,7 @@ \section{Expressions}

\begin{grammar}
<expression> ::= <assignableExpression> <assignmentOperator> <expression>
\alt <conditionalExpression> <cascadeSection>*
\alt <conditionalExpression> <cascadeSequence>?
\alt <throwExpression>

<expressionWithoutCascade> ::= \gnewline{}
Expand Down Expand Up @@ -9485,10 +9488,14 @@ \subsubsection{Cascaded Invocations}
where $e$ is an expression and \metavar{suffix} is a sequence of operator, method, getter or setter invocations.

\begin{grammar}
<cascadeSection> ::= `..' (<cascadeSelector> <argumentPart>*)
<cascadeSequence> ::= (`?..' \alt `..') <cascadeSection> <cascadeSectionContinuation>*

<cascadeSection> ::= (<cascadeSelector> <argumentPart>*)
\gnewline{} (<assignableSelector> <argumentPart>*)*
\gnewline{} (<assignmentOperator> <expressionWithoutCascade>)?

<cascadeSectionContinuation> ::= `..' <cascadeSection>

<cascadeSelector> ::= `[' <expression> `]'
\alt <identifier>

Expand Down Expand Up @@ -11502,6 +11509,9 @@ \subsection{Assignable Expressions}

<assignableSelector> ::= <unconditionalAssignableSelector>
\alt `?.' <identifier>
\alt `?[' <expression> `]'
\alt `!'
Copy link
Member

Choose a reason for hiding this comment

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

I don't think x! should be assignable. Put it into <selector> instead.
It might just be a bad name. You can write garbage like x..foo() = 42 with the existing grammar too, it'll be disallowed by the equivalence to x.foo() = 42, which is not valid.

Maybe we can use a grammar like:

<cascadeSequence> ::= (`?..' \alt `..') <cascadeSection> (`..' <cascadeSection>)*

<cascadeSection> ::= <cascadeSelector> (<cascadeAssignment> | <selector>* (<assignableSelector> <cascadeAssignment>)?)

<selector> ::= `!' 
  \alt <argumentPart>
  \alt <assignableSelector>

<assignableSelector> ::= (`?.' \alt `.') <identifier>
  \alt `?.'? `[' expression `]'

<cascadeAssignment> ::= <assignmentOperator> <expressionWithoutCascade>

(should we allow <incrementOperator> in <cascadeAssignment> too, so you can do foo..bar++..baz = 42 and not just foo..bar += 1..baz = 42?)

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 don't think x! should be assignable. Put it into <selector> instead.

If you do this, then you can't write x!.y = 3, which I think we want to be able to write. Unless I'm missing something? But I think the x!.y can't be derived as a selector, you have to go through assignableSelector. So I was planning on adding this here, and then disallowing things like x! = 3 semantically.

You can write garbage like x..foo() = 42 with the existing grammar too,

How does this derive? I don't see how x..foo() gets derived as an <assignableExpression>.

Copy link
Member

@lrhn lrhn Apr 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

If you change selector to:

<selector> ::= `!'
 \alt <argumentPart>
 \alt <assignableSelector>

and either assignableSelectorPart to:

<assignableSelectorPart> ::= (<argumentPart> | `!')* <assignableSelector>

(from <assignableSelectorPart> ::= <argumentPart>* <assignableSelector>)
or change that and assignableExpression to:

<assignableeExpression> ::= <primary> <assignableSelectorPart> 
  \alt \SUPER{} <unconditionalAssignableSelector>
  \alt <constructorInvocation> <assignableSelectorPart>
  \alt <identifier>

<assignableSelectorPart> ::= <selector>* <assignableSelector>

then you can derive x!.y = 3.

You are correct that just changing <selector> is not enough because assignableSelectorPart doesn't use selector.

About ..foo() = 42, the grammar for cascadeSection is

<cascadeSection> ::= `..' (<cascadeSelector> <argumentPart>*)
  \gnewline{} (<assignableSelector> <argumentPart>*)*
  \gnewline{} (<assignmentOperator> <expressionWithoutCascade>)?

and with:

  • <cascadeSelector> ↦ <identifier> ↦ foo
  • <argumentPart> ↦ <arguments> ↦ ( )
  • <assignmentOperator> ↦ =
  • <expressionWithoutCascade> ↦* 42

you can derive x .. foo () = 42 by having one <argumentPart>, zero (<assignableSelector> <argumentPart>*) and one (<assignmentOperator> <expressionWthoutCascade>) as allowed by the multiplicities.
There is no <assignableExpression> involved.

The problem is that <argumentPart> is a selector and not an assignable selector, but the cascade grammar allows inserting it before the assignment. The issue is the cascade grammar, and as you say the <assignableExpression> doesn't have that issue.
I think my suggestion above is better (but would obviously like someone else to check that it still allows all the right things).

The code is rejected by our tools anyway, even though it satisfies the grammar. It is then treated as equivalent to x.foo() = 42, which is itself invalid. Example:

main() {
  42..toString() = 4;
}

gives the CFE error:

casc.dart:2:18: Error: Can't assign to this.
  42..toString() = 4;

and the analyzer error:

Analyzing casc.dart...
  error • Missing selector such as '.<identifier>' or '[0]' at casc.dart:2:5 • missing_assignable_selector
  error • A value of type 'int' can't be assigned to a variable of type 'String' at casc.dart:2:20 • invalid_assignment
2 errors found.

(The second error is spurious—there is no variable of type string in the program—which suggests that the analyzer isn't entirely comfortable with this misuse of syntax).

Copy link
Member Author

Choose a reason for hiding this comment

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

@lrhn If I do as you suggest, then I don't think you can have ! in a cascade section anymore, right?

a..foo()!.bar=3;

I think this cascade parses with my grammar as:

cascadeSelector -> identifier -> foo
argumentPart* -> argumentPart -> ()
assignableSelector -> !
assignableSelector -> .bar
assignmentOperator -> =
expressionWithoutCascade -> conditionalExpression -> ifNullExpression -> logicalOrExpression -> logicalAndExpression -> equalityExpression -> relationExpression -> bitwiseOrExpression -> bitwiseXOrExpression -> bitwiseAndExpression -> shiftExpression -> additiveExpression -> multiplicativeExpression -> unaryExpression -> StayWithMeAlmostThere -> primary -> literal -> numericLiteral

How does it parse with yours?

Copy link
Member

@lrhn lrhn May 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Let's pick the first grammar I proposed:

<cascadeSequence> ::= (`?..' \alt `..') <cascadeSection> (`..' <cascadeSection>)*

<cascadeSection> ::= <cascadeSelector> (<cascadeAssignment> | <selector>* (<assignableSelector> <cascadeAssignment>)?)

<selector> ::= `!'     /// <--- the ! is here
  \alt <argumentPart>
  \alt <assignableSelector>

<assignableSelector> ::= (`?.' \alt `.') <identifier>
  \alt `?.'? `[' expression `]'

<cascadeAssignment> ::= <assignmentOperator> <expressionWithoutCascade>

Then the cascade section of

a...foo()!.bar = 3

parses as

  • cascadeSection ↦ cascadeSelector selector* assignableSelector cascadeAssignment
  • cascadeSelector ↦ identifier (foo)
  • selector* ↦ selector selector
  • selector ↦ argumentPart ↦* ( )
  • selector ↦ !
  • assignableSelector ↦ . identifier (bar)
  • cascadeAssignment ↦ = expressionWithoutCascade
  • expressionWithoutCascade ↦* integerLiteral (3)

We only want the actually assignable selectors to be assignable, and ! does not introduce a LHS, so it should not be an assignable selector. Only .id, [e] and ?.[e] are assignable because it's the only suffixes that can denote a setter (counting operator[]= as a settter).

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I've taken your suggestion, PTAL.


\end{grammar}

\LMHash{}%
Expand Down Expand Up @@ -14121,15 +14131,18 @@ \subsection{Static Types}
}

\begin{grammar}
<type> ::= <functionTypeTails>
\alt <typeNotFunction> <functionTypeTails>

<type> ::= <functionType> `?'?
\alt <typeNotFunction>

<typeNotFunction> ::= <typeNotVoidNotFunction>
\alt \VOID{}
<typeNotVoid> ::= <functionType> `?'?
\alt <typeNotVoidNotFunction>

<typeNotVoidNotFunction> ::= <typeName> <typeArguments>?
\alt \FUNCTION{}
<typeNotFunction> ::= \VOID{}
\alt <typeNotVoidNotFunction>

<typeNotVoidNotFunction> ::= <typeName> <typeArguments>? `?'?
\alt \FUNCTION{} `?'?
leafpetersen marked this conversation as resolved.
Show resolved Hide resolved

<typeName> ::= <typeIdentifier> (`.' <typeIdentifier>)?

Expand All @@ -14140,13 +14153,10 @@ \subsection{Static Types}
<typeNotVoidNotFunctionList> ::= \gnewline{}
<typeNotVoidNotFunction> (`,' <typeNotVoidNotFunction>)*

<typeNotVoid> ::= <functionType>
\alt <typeNotVoidNotFunction>

<functionType> ::= <functionTypeTails>
\alt <typeNotFunction> <functionTypeTails>

<functionTypeTails> ::= <functionTypeTail> <functionTypeTails>
<functionTypeTails> ::= <functionTypeTail> `?'? <functionTypeTails>
\alt <functionTypeTail>

<functionTypeTail> ::= \FUNCTION{} <typeParameters>? <parameterTypeList>
Expand Down
Loading