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-aware member access needlessly an error with special receiver types? #711

Closed
eernstg opened this issue Nov 28, 2019 · 5 comments
Closed
Labels
nnbd NNBD related issues question Further information is requested

Comments

@eernstg
Copy link
Member

eernstg commented Nov 28, 2019

Thanks to @sgrekhov for spotting this issue. Consider the following program:

void main() {
  Null a = null;
  a?.toString();
}

Following the nnbd specification, the invocation of toString is a compile-time error: The null-shorting member access ?. is checked as if the type of the receiver were non-nullable (because it should not be an error to access a member on a receiver that might be null when that's the whole point of the mechanism), and more specifically: It is NonNull(T) where T is the receiver type.

However, we might want to introduce a notion of "the non-nullable version of a type" which is slightly different for this purpose:

NonNull(Null) is Never, but if n has type Never then n.toString() is a compile-time error. The statement is guaranteed to simply do nothing (except that the evaluation of the receiver could have side-effects, and might not terminate), but there is nothing wrong in evaluating a and obtaining the null object and then skipping .toString(). So it's slightly odd that it is an error.

Another place where the NonNull function may not deliver the most useful result is FutureOr<T>: That type is returned unchanged by NonNull. Again, the anomaly is small, because it's difficult to invoke any other members than the ones from Object, even on a receiver of type FutureOr<Future<T>> for some T.

So do we just keep this special case as it is? @munificent, @lrhn, @leafpetersen, WDYT?

@eernstg eernstg added question Further information is requested nnbd NNBD related issues labels Nov 28, 2019
@lrhn
Copy link
Member

lrhn commented Dec 2, 2019

I have no problem keeping the error. If we disallow member access on Never (because it's dead code), then so is this.

@munificent
Copy link
Member

munificent commented Dec 2, 2019

So it's slightly odd that it is an error.

I think this is a great place for a warning. The code is not actively harmful and its behavior is well-defined, but also very clearly does not do anything useful.

If it were me, I'd have all dead code, unreachable code, unused variable, unused import, etc. diagnostics be warnings. And, like Lasse, I consider this to be dead code.

@eernstg
Copy link
Member Author

eernstg commented Dec 3, 2019

The underlying issue is actually that we need a way to talk about a type which is obtained by stripping Null off of an existing type.

We discussed introducing a suffix ! operator on types to express that exact concept, but dropped it.

We also don't have the concept in the "spec model" of types that we use to specify the subtyping relation. We do have other non-denotable types, e.g., the subtype rules talk about types of the form X & S where X is a type variable.

I think we need to introduce a suffix ! operator in said spec model of types, especially when we specify the enhanced promotion mechanism.

With that in place, we could replace the rule that relies on NonNull(T) by a rule that relies on T!:

For the purposes of errors and warnings, the null aware operators ?., ?..,
and ?.[] are checked as if the receiver of the operator had non-nullable type.
More specifically, if the type of the receiver of a null aware operator is T, then
the operator is checked as if the receiver had type T!.

Consider a variable x of type X, which is a potentially nullable type variable. After a null check we can give it type X! which is known to be a non-nullable type, as opposed to the situation where we use NonNull(X) and obtain a plain X, thus forgetting that x can't be null. Similarly for a variable of type FutureOr<int?>: After a null check it may be promoted to FutureOr<int?>!, which is not otherwise expressible, and the current rule yields FutureOr<int?> and forgets that it isn't null.

However, this doesn't change the situation for the receiver type Null: Null! is still Never, so if we're consistent we still make a?.m() from the example an error.

So, @munificent, I believe the best way to integrate your preferred approach consistently into the language would be to eliminate the rule that every member access on Never is an error, and then the error in this example disappears just like all the others (and we get a warning about unreachable code for the invocation itself).

@lrhn
Copy link
Member

lrhn commented Dec 3, 2019

If member accesses on Never are generally warnings, not errors, then so should this be. There is nothing special in this particular case, it's just a use of an "object" which is known to be of an empty type.

@leafpetersen
Copy link
Member

We are changing the specification of errors on Never to allow all member accesses. This code will be accepted, but potentially subject to dead code warnings.

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
Projects
None yet
Development

No branches or pull requests

4 participants