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

Null-shorting to the left by desugaring? #1081

Closed
eernstg opened this issue Jul 9, 2020 · 7 comments
Closed

Null-shorting to the left by desugaring? #1081

eernstg opened this issue Jul 9, 2020 · 7 comments
Labels
nnbd NNBD related issues question Further information is requested specification

Comments

@eernstg
Copy link
Member

eernstg commented Jul 9, 2020

Cf. dart-lang/sdk#42379.

Null-shorting allows expressions like e?.a().b().c() because execution of both a(), b(), and c() is omitted in the case where e evaluates to null.

However, (e?.a()).b().c() is an error, because e?.a() may yield null, and null-shorting does not propagate through parentheses (so we must use (e?.a())?.b().c()).

In general, null-shorting proceeds to the right and includes only constructs derived from <selector> (that is, member invocations like .getter, .method(...)), so we can't have e1?.a().b().c() + e2 and expect null-shorting to skip the invocation of + if e1 evaluates to null. Instead, e1?.a() + e2 is an error because the receiver may be null.

It turns out that we have a couple of exceptions:

class C {
  C e() => this;
  C operator -() => this;
  C operator ~() => this;  
  C operator +(C other) => this;
  int f = 1;
}

void main() {
  C? c = new C();
  -c?.e();
  ~c?.e();
}

The above program is accepted by the analyzer (SDK 2.9.0-edge.2ed302bfbbbad1d10f6c839c504ae3871fc96dc2), but rejected by the common front end.

Acceptance of the program may be justified by the following sentence in the language specification:

Any other expression of the form op e is equivalent to the
method invocation e.op().

This rule actually allows null-shorting to proceed from a subexpression of e and all the way up to, and including, .op(). This affects unary - and ~.

However, the language specification uses a similar approach for <relationalExpression>, 4 variants of <bitwise...Expression>, <shiftExpression>, <additiveExpression>, and <multiplicativeExpression>.

None of our tools use those approaches to let null-shorting propagate through all those binary operators.

The discussions about null-shorting did certainly include discussions about many of these constructs, and we decided that null-shorting should be somewhat limited in order to be predictable and readable. I believe we agreed to exclude expressions like c?.a() + e2 from skipping +.

We can change the specification to work in that manner by adding parentheses around the receiver expression:

A relational expression of the form e1 op e2 is equivalent to the
method invocation (e1) .op(e2)

I expect this change to be uncontroversial. It is non-breaking because the analyzer and CFE already behave as if we had done this.

However, we may then also wish to change the rule about <unaryExpression>, for consistency, as follows:

Any other expression of the form op e is equivalent to the
method invocation (e).op().

This would turn -c?.e() and ~c?.e() in the example program into compile-time errors.

If we do this then the analyzer would emit new errors, but these errors are already emitted by the front end, and we have previously considered such changes to be non-breaking.

@natebosch, @munificent, @stereotype441, @lrhn, @leafpetersen, @jakemac53, WDYT?

@lrhn
Copy link
Member

lrhn commented Jul 9, 2020

Agree on making it explicit that it's a compile-time error.

We also have --x?.y as a potential issue. I currently works because it's defined to be equivalent to x?.y -= 1, which is supposed to work.

@eernstg
Copy link
Member Author

eernstg commented Jul 9, 2020

Yes, I considered --x?.y as well, and that is indeed rather convoluted (esp. because it can now be --e1?.s1().s2().s3().y such that there is no ? in the location where the language specification expects it), but this is something that we have discussed and, I think, accepted. Also, the spec language does cover this in a way that does not require any changes (other than that missing ?).

@Cat-sushi
Copy link

I think null-shorting is a syntax sugar for chain of ?., and should be limited for ?..
Because, for me, even e1?.a.b is a little confusing, and operation e1?.a + e2 have different syntax from e1?.a.+(e2).

@lrhn
Copy link
Member

lrhn commented Jul 10, 2020

@Cat-sushi
Null-shortening ?. is actually not syntactic sugar for a chain of ?.s. We made the change because with null safety and non-nullable types, we needed something stronger.

Example:

class C {
  int foo() => 42;
  int? bar() => null;
}
main() {
  C? c = C();
  var v1 = c?.foo().toString();  // "42"
  var v2 = c?.bar().toString();  // "null" **
  var v3 = c?.bar()?.toString();  // null
}

If null shortening was just inserting ?. in the rest of the chain, then you could not get the v2 result, where you do call toString on the nullable result of bar().

That said, the range of a ?. should still be limited to where a ?. could occur.

@leafpetersen
Copy link
Member

We still have disagreement in the tools here. @eernstg Can you move forward with specify this and land tests?

@eernstg
Copy link
Member Author

eernstg commented Oct 8, 2020

Test up for review here, landed as dart-lang/sdk@2d92a26. There are no implementation issues, it is already implemented.

dart-bot pushed a commit to dart-lang/sdk that referenced this issue Oct 8, 2020
This verifies that the operators `unary-` and `~` do not participate
in null-shorting. Cf. dart-lang/language#1081.

Change-Id: Iba446724f00c1c4aaedb4be4425e69fb52cdb5c9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/166627
Reviewed-by: Lasse R.H. Nielsen <lrn@google.com>
Commit-Queue: Erik Ernst <eernst@google.com>
eernstg added a commit that referenced this issue Oct 9, 2020
eernstg added a commit that referenced this issue Oct 12, 2020
Specify that the null shorting transformation does not consider operator invocations as member invocations, cf. #1081. This implies that a construct like `-a?.b` is an error because `a?.b` can be null and `-` is not defined on a nullable receiver. If, following the language specification, we consider `-a?.b` to be syntactic sugar for `a?.b.unaryMinus()`, where `unaryMinus` is an instance method with the same behavior as `unary-`, we get a different behavior: `-a?.b` would then be allowed, and `-` / `unaryMinus()` would be skipped if `a` is null, so we need to say that this equivalence does not apply for null shorting. The same situation arises for other operators, e.g., additive and multiplicative ones.
@eernstg eernstg closed this as completed Oct 13, 2020
@eernstg
Copy link
Member Author

eernstg commented Oct 15, 2020

Spec updated in #1256.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nnbd NNBD related issues question Further information is requested specification
Projects
None yet
Development

No branches or pull requests

4 participants