Fixes #2490. Add more await tests for extension types

[sgrekhov]

No description provided.

[eernstg]

Looks good! A few tests have issues, though, in particular with intersection types.

[sgrekhov]

Updated. Please take another look

[eernstg]

I think this would have to be something like x is NumET: We don't get a promotion of x to a non-subtype of FutureOr<NumET>, and then we aren't testing anything of the form X & B.

[sgrekhov]

Changed to

extension type NumET(num _) implements num {}
extension type IntET(int _) implements NumET {}

This should work, shouldn't it?

[eernstg]

Not yet: The type of x is initially X extends FutureOr<NumET>. We then test for int, but int is not a subtype of FutureOr<NumET> and hence there's no promotion and no intersection type.

However, NumET <: FutureOr<NumET> because T <: FutureOr<T> for any T (including extension types), so we could have if (x is NumET) ... and that would promote the type of x to X & NumET. Similarly, we could have if (x is IntET) ... if we have IntET <: NumET (as in the new code).

NumET and IntET are incompatible with await, but that's OK: If we await x then we just determine that NumET/IntET do not derive a future type, but X does, and then await x is OK, with type flatten(X) == flatten(FutureOr<NumET>) == NumET.

[sgrekhov]

NumET and IntET are incompatible with await, but that's OK: If we await x then we just determine that NumET/IntET do not derive a future type, but X does, and then await x is OK, with type flatten(X) == flatten(FutureOr<NumET>) == NumET.

Sorry, I don't quite understand. Do you want to sat that

if (x is IntET) {
  await x;
}

is OK? No error? But, according to the definition

• T is X & B, and either:
  • B is incompatible with await, or

In case of x is IntET, IntET is incompatible with await, criteria met, so await x should be a compile-time error.

[eernstg]

Ah, you're right. We have a slight inconsistency because 'incompatible with await' is more aggressive than flatten, so we never get around to try to invoke flatten on the type of x because it is already an error, as you mention, according to the 'incompatible' rule to have await x.

The underlying question is "do we want to make it an error to have await e when the type of e is X & B where B 'is bad' but X is 'good'?". The 'incompatible' rule says "yes, don't even bother to look at X", but flatten allows us to disregard B if it doesn't derive any future types, and then use X.

So the question is whether we can actually have this situation at all:

• We do await x.
• The static type of x is X & B.
• B is an extension type which is compatible with await.
• B does not derive a future type.
• So we use X in flatten, and it does derive a future type.

I don't think we can find a type B with those properties.

So this basically means that we can't use the 'otherwise' case in the rule about flatten on an intersection type X & B where B is an extension type.

But then we'd just have to test that we do get the compile-time error when B is an extension type that does not implement Future<T> for any T. Perhaps that's already done in some other test?

Next, we can test that the first case is used in the flatten rule about X & B when X does not derive a future type, but B does (perhaps X extends Object and B == NumFutureET).

[sgrekhov]

But then we'd just have to test that we do get the compile-time error when B is an extension type that does not implement Future<T> for any T. Perhaps that's already done in some other test?

Below in this test we have if (x is IntET) {

Next, we can test that the first case is used in the flatten rule about X & B when X does not derive a future type, but B does (perhaps X extends Object and B == NumFutureET).

Added static_analysis_extension_types_A08_t11.dart which checks it. In case X extends Object X is compatible with await, so I used X extends ObjectET

[eernstg]

Still a couple of issues. I've commented on several cases where the description is about intersection types, but we don't have an expression whose type is an intersection type. In general, we just need to perform the type test on a subtype of the current type of the variable at that point.

[sgrekhov]

Updated. Please review, there are some places where I'm not 100% sure

[sgrekhov]

Changed to

extension type NumET(num _) implements num {}
extension type IntET(int _) implements NumET {}

This should work, shouldn't it?

[eernstg]

This one still has some issues.

[eernstg]

Not yet: The type of x is initially X extends FutureOr<NumET>. We then test for int, but int is not a subtype of FutureOr<NumET> and hence there's no promotion and no intersection type.

However, NumET <: FutureOr<NumET> because T <: FutureOr<T> for any T (including extension types), so we could have if (x is NumET) ... and that would promote the type of x to X & NumET. Similarly, we could have if (x is IntET) ... if we have IntET <: NumET (as in the new code).

NumET and IntET are incompatible with await, but that's OK: If we await x then we just determine that NumET/IntET do not derive a future type, but X does, and then await x is OK, with type flatten(X) == flatten(FutureOr<NumET>) == NumET.

[eernstg]

We still can't promote to int, because that isn't a subtype of NumET.

[sgrekhov]

Indeed... Removed this

[eernstg]

Can't promote to FutureOr<IntET>.

[eernstg]

Can't promote x because FutureOr<IntET> is not a subtype of num.

[sgrekhov]

Fixed

[eernstg]

Added a comment.

[eernstg]

Ah, you're right. We have a slight inconsistency because 'incompatible with await' is more aggressive than flatten, so we never get around to try to invoke flatten on the type of x because it is already an error, as you mention, according to the 'incompatible' rule to have await x.

The underlying question is "do we want to make it an error to have await e when the type of e is X & B where B 'is bad' but X is 'good'?". The 'incompatible' rule says "yes, don't even bother to look at X", but flatten allows us to disregard B if it doesn't derive any future types, and then use X.

So the question is whether we can actually have this situation at all:

• We do await x.
• The static type of x is X & B.
• B is an extension type which is compatible with await.
• B does not derive a future type.
• So we use X in flatten, and it does derive a future type.

I don't think we can find a type B with those properties.

So this basically means that we can't use the 'otherwise' case in the rule about flatten on an intersection type X & B where B is an extension type.

But then we'd just have to test that we do get the compile-time error when B is an extension type that does not implement Future<T> for any T. Perhaps that's already done in some other test?

Next, we can test that the first case is used in the flatten rule about X & B when X does not derive a future type, but B does (perhaps X extends Object and B == NumFutureET).

[sgrekhov]

Thank you! Updated once again. Please review

[sgrekhov]

Indeed... Removed this

[sgrekhov]

But then we'd just have to test that we do get the compile-time error when B is an extension type that does not implement Future<T> for any T. Perhaps that's already done in some other test?

Below in this test we have if (x is IntET) {

Next, we can test that the first case is used in the flatten rule about X & B when X does not derive a future type, but B does (perhaps X extends Object and B == NumFutureET).

Added static_analysis_extension_types_A08_t11.dart which checks it. In case X extends Object X is compatible with await, so I used X extends ObjectET

[sgrekhov]

Fixed

[eernstg]

Just one more thing: I think there is still one ? that prevents promotion.

[eernstg]

LGTM!