dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.27k stars 1.58k forks source link

[extension types] Confusing nullability related error message. #56724

Open modulovalue opened 2 months ago

modulovalue commented 2 months ago

Consider:

void main() {
  print("foo");
}

final a = <N extends ExtensionType>(
  final N n,
) => n.pointer;
//     ^^^^^^^
// Analyzer: The property 'pointer' can't be unconditionally accessed because the receiver can be 'null'.

extension type ExtensionType(int pointer) {}

This program compiles and outputs "foo", but the analyzer reports an error message.

I expected the program to either:

This seems to be a bug, presumably in the analyzer.

dart-github-bot commented 2 months ago

Summary: The analyzer incorrectly reports an error about accessing pointer in an extension type, even though the code compiles and runs without issue. This suggests a potential bug in the analyzer's handling of nullability within extension types.

eernstg commented 2 months ago

Good catch, this kind of situation has probably not been explored very well until now! However, I think it can be treated in a way that is based on existing rules and behaviors.

We have the general rule that the interface of a type variable X extends B is the same as the interface of its bound B. Given that extension type members are statically resolved this would imply that there is simply no difference between the behavior of a receiver whose static type is N and a receiver whose static type is the bound ExtensionType:

For an extension type member invocation, it's statically resolved to the same code in the two cases. For an OO dispatched instance member invocation (that is, an invocation of an instance member declared by a class, mixin, enum, etc), the behavior is determined by the run-time type of the representation object.

The use of a type variable does enable some amount of expressive power:

void main() {
  a(ExtensionSubtype(14));
}

final a = <N extends ExtensionType>(
  final N n,
) => print((n as ExtensionType).pointer);

extension type ExtensionType(num pointer) {}

extension type ExtensionSubtype._(int pointer) implements ExtensionType {
  ExtensionSubtype(int i): this._(2 * i);
}

The upcast n as ExtensionType is intended to ensure that n is treated as having the interface that I think it should have, namely that of ExtensionType (so that's just something we'd do now, to get the desired behavior of the analyzer). However, the run-time value of N is int, not num, which means that someExpression as N has a different effect than someExpression as ExtensionType. So we can use this to impose a stronger run-time constraint on the type of the representation object than that which is expressed by the extension type which is used as a bound.

Similarly, the use of ExtensionSubtype at the call site ensures that the representation object actually has that stronger type.

At first I thought that it would simply be a useless exercise to use an extension type as a type parameter bound, but it will do certain things that might not be expressible otherwise! :smile:

With respect to the need to adjust the analyzer (the common front end seems to handle it already), it should be changed such that extension type members can be invoked on a receiver whose static type is a type variable which is bounded by an extension type, even though the receiver type is potentially nullable.

lrhn commented 2 months ago

We've seen a number of similar bugs before. Before extension types, the only members a nullable or potentially nullable type could have were those of Object, and then extension methods could be added later.

A potentially nullable extension type can have more members, those declared by the extension type itself (including its representation variable getter), or inherited from another potentially nullable extension type.

Seems like the analyzer still contains a place where it checks "Is the type non-nullable? If not, you can't access members on it." even though that rule is not always true any more. It does recognize that the member is there, otherwise it would have said that there was no pointer member to access.

And as @eernstg says, a workaround is to write (n as ExtensionType).pointer, which is an up-cast and shouldn't cost anything at runtime. (But also shouldn't be needed.)