Should implicit downcasts promote in null safety mode?

[stereotype441]

I just noticed a curious inconsistency in flow analysis.

Explicit casts from dynamic to another type cause type promotion, e.g.:

f(String s) {}

g(dynamic d) {
  f(d as String); // `d as String` causes `d`'s type to be promoted. 
  print(d.isEven); // static error: `d` is promoted to `String`, and `String` does not support `isEven`
}

But implicit downcasts don't:

f(String s) {}

g(dynamic d) {
  f(d); // Implicitly downcasts to `String`, but does not promote.
  print(d.isEven); // no static error; this is a dynamic invocation.
}

Both behaviors are sound, and both behaviors are implemented consistently between the analyzer and the CFE. So it's not necessary for us to do anything. But it seems a little strange that an implicit downcast and an explicit downcast would be treated so differently. Should we consider changing flow analysis so that f(d) also promotes?

[lrhn]

I worry about the type of a variable changing with no syntactic hint. That feels like something that can be very hard to debug.

It's only for dynamic now, since it's the only remaining type which is implicitly downcastable. I don't think we should implicitly take away the property of being dynamic. If you have an object implementing unrelated types A and B in a dynamic variable, and you pass it to something expecting an A, you suddenly can't call a B method on the variable.

So, ... all in all, I'd prefer not to promote on implicit downcasts. You don't get anything for free with dynamic.

[leafpetersen]

+1. This seems like it would lead to really surprising behavior to me.

[stereotype441]

Ok, that sounds like enough of a consensus to me. I'll make sure that we have tests to lock in this behavior (since it kind of got implemented by accident), and then once those land I'll close this issue.

[eernstg]

I agree that we should be careful about invisible promotions. Given that I've been loudly worried about this specific issue, I can't help mentioning that we do have some situations where promotion takes place with no syntactic hint:

import 'someoneElsesLibrary.dart'; // Exports `int foo()`.

void main() {
  int? i;
  i = foo();
  i.isEven; // OK.

  int? j = ...;
  j!.toString();
  j.isEven; // OK.
}

Similarly, in a non-trivial function body where int is at some point made a type of interest for i before i = foo();, the promotion would take place even if i has another declared type like Object or num. In this case there is no syntactic hint at the location where the promotion occurs, and the construct that makes int a type of interest may be far away and hard to spot.

What's the most consistent approach?

We could treat the success of an implicit downcast from dynamic in a similar way as the implicit type evidence provided by an assignment. We would then promote d if the dynamically checked type is a type of interest for d. This is again similar to the approach taken with ! where we promote to non-null based on the success of the dynamic check, in a situation where the resulting type is of interest already.

void f(dynamic d) {
  if (d is int) { ... }; // `int` now of interest.
  g(d);
  d.isEven('Wrong argument list shape'); // Do we want the error here? ..
  d = h();
  d.isEven('Again'); // .. considering that we do get it here?
}

void g(int i) {}
int h() => 0;

It seems consistent to me if we basically use every opportunity to promote to a type of interest, even though these promotions will generally not have a syntactic marker.

[lrhn]

I think the examples here are significantly different from an implicit downcast.

An assignment changes the value of a variable to something under your control. That's a reasonable place to also change the type to the type of the assigned value.
An explicit v! is equivalent to an v as SomeType, which is again equivalent to v is SomeType ? v /*promoted by is*/ : throw ....
Again, you asked for it.

The implicit downcast does not assign a new value, and it doesn't have an explicit cast to signify that something happens here.
Also, it's a cast which depends on the context type, something which can change without your involvement (it's a non-breaking change to widen the parameter types of a static function. But then, it's also a non-breaking change to narrow the return type, so the assignment above isn't as clear any more).

[stereotype441]

Ok, that sounds like enough of a consensus to me. I'll make sure that we have tests to lock in this behavior (since it kind of got implemented by accident), and then once those land I'll close this issue.

https://dart-review.googlesource.com/c/sdk/+/175902 tests the current behavior so that we won't change it by accident in the future

[eernstg]

An assignment changes the value of a variable to something under your control

I think the "best" kind of promotion we have is x is T and x as T, because they are explicit: First, the target type occurs in the source code, available for the reader, and it will not change due to a pub upgrade; secondly, promotion will typically occur.

I actually think that we would help developers by making it more strict, e.g., by pointing out explicitly that promotion failed at a specific type test or type cast, as soon as there is a compile-time error that would not have occurred in case the promotion had taken place. We could also warn at myInstanceVariable is T, and tell the developer to use var x: myInstanceVariable is T (if we choose to support binding expressions, #1210) or myInstanceVariable is late T (if we choose to support dynamically checked promotion, #1188). Developers would then be able to know with even greater certainty that is and as mean promotion.

In contrast, many (surely most) assignments do not promote, and promoting assignments are implicit in other ways as well: Assignments don't make the target type visible, and the target type may change arbitrarily on pub upgrade. This makes it harder for the reader to tell that there is a promotion, and what the resulting type of the given variable will be.

We have previously made the choice (a good choice, I think) to limit the applicability of implicit promotions, such that they can only promote to a type of interest, that is, a type which has actually been mentioned nearby, in connection with that variable.

So the question now is whether we'd want to use the same approach (promote, but only to type of interest) also for dynamic checks applied to local variables of type dynamic.

I agree that this adds to the number of locations where promotion can take place implicitly, but let's consider the trade-off:

@lrhn wrote:

very hard to debug

@leafpetersen wrote:

really surprising behavior

That's a real danger with any implicit promotion.

However, if we promote more variables of type dynamic in more lines of code, then we are changing more pieces of dynamically type checked code to statically checked code.

Do you have some good examples where this makes the code very hard to debug?

Also, if the variable stays dynamic then it enables a style of programming where we rely on the variable to have a union type, because we use features of two or more types, say, A and B, and then it would break if we promote the variable to A or B, because we want both. Is that style of coding really important enough to justify preservation?

[stereotype441]

I landed dart-lang/sdk@c171f92, which tests the current behavior (implicit downcasts don't promote). I'm going to leave the issue open in case folks want to discuss more.