There's no good way to promote an argument #35524

Hixie · 2018-12-30T20:40:37Z

(This bug assumes a world with implicit-casts: false.)

If I have a method which takes an argument of unknown type, there's currently no clean way to write the code that handles that argument in a type-safe way, as far as I can tell.

void foo(Object bar) {
  if (bar.runtimeType == runtimeType) {
    // ok we know it's the same type as us, but neither the analyzer nor the compiler agree
    final Bar<T, R, S> typedBar = bar as Bar<T, R, S>; // this line is really ugly.
    // not we can use typedBar, but that's ugly.
    typedBar.la(1);
    typedBar.la(2);
    typedBar.la(3);
  }
  // ...
}

void foo(dynamic bar) {
  if (bar.runtimeType == runtimeType) {
    // ok we know it's the same type as us, but neither the analyzer nor the compiler agree
    // so we get no type checking here
    bar.la(1);
    bar.la(2);
    bar.laa(3); // oops typo, nobody will notice until runtime
  }
  // ...
}

void foo(Object bar) {
  if (bar.runtimeType == runtimeType) {
    // ok we know it's the same type as us, but neither the analyzer nor the compiler agree
    assert(bar is Bar<T, R, S>); // this doesn't do anything for the compiler or the analyzer
    bar.la(1); // the compiler doesn't allow this
    bar.la(2);
    bar.la(3);
  }
  // ...
}

What is the preferred way to write this code?

This comes up all the time e.g. in operator == implementations, or when handling Json data or other structured data parsers like the StandardMessageCodec logic in Flutter.

The text was updated successfully, but these errors were encountered:

lrhn · 2019-01-01T19:53:12Z

My first recommendation is (as usual) to not use == runtimeType because it just doesn't do very much for you.

The runtimeType is a reflexive feature, and there is no promotion based on such equality checks.
The only operation that can do type promotion is an is check.

If the third example is correct and this and bar really are Bar<T, R, S>, then you can do:

void foo(Object bar) {
  if (bar.runtimeType == this.runtimeType && bar is Bar<T, R, S>) {
    ...
  }
}

That 's the only way to promote an existing variable. Otherwise you need to introduce a new variable like you did in the first example.

Hixie · 2019-01-01T21:35:24Z

As far as I can tell, == runtimeType is the only correct way to implement operator ==. (Actually we usually use if (other.runtimeType != runtimeType) return false;, but same idea.)

It's not always runtimeType, though. Another example I have here is one where some other invariant (unrelated to runtimeType) being true allows us to know for sure that the argument is of a particular type, but converting it from its current static type to the known-to-be-true actual type so that it can be handled requires assigning it to another variable with an explicit cast, which ends up being really ugly (option 1 above), unless I add this is check. The is check is pretty ugly too though.

Also, in general, the is trick isn't a good solution because it results in extra code at runtime that is provably redundant. It also conveys the wrong message to the person reading the code: it looks like it's saying that the type might not be right, which then leads to the question about what should happen if it isn't right, etc. I would much rather have an assert than an is (the third example above, for instance).

leafpetersen · 2019-01-03T00:24:27Z

Also, in general, the is trick isn't a good solution because it results in extra code at runtime that is provably redundant

This is only sometimes going to be true in aggregate. The is approach provides the compiler (especially the AOT compiler!) with a lot of information about the shape of the argument that it otherwise has to test for on each invocation on that argument. If we promote from an assert, then the compiler would have to reinsert the invocation tests when running in production mode when asserts are elided. @mraleph can comment better on whether eliding the is check is actually a net win, but I'd be surprised if it's noticeable (esp in AOT).

I do think that providing a way to do what you're trying to do more succinctly and in a way that both the analyzer and the compiler can take advantage of would be a good thing though.

Hixie · 2019-01-04T00:49:00Z

This is only sometimes going to be true in aggregate

Sure. This bug is about the cases where it's true.

If we promote from an assert, then the compiler would have to reinsert the invocation tests when running in production mode when asserts are elided

Or we could crash, that'd be fine too. That's what would happen in C++, for example.

Anyway, I'd be fine with release builds having a very cheap version of the assert. We could even have dedicated syntax for this kind of check.

  alwaysAssert(foo is Bar);

In debug builds (if false) it could throw and so on, in release builds it could just abort the program.

Hixie · 2019-02-21T23:42:31Z

Or maybe:

  promote foo as Bar;

lrhn · 2019-02-26T08:48:58Z

There is currently no way to derive type promotion from runtimeType checks. There likely won't be, exactly because runtimeType can be overridden. A clever compiler with access to the entire program might be able to recognize that no class overrides runtimeType in a way that can possibly make it pass this check, and then realize that a following type check is redundant, but that's completely speculative.

If you want promotion, you will have to do an actual type check. Whether you do:

Subtype newVar = oldVar;

or

if (oldVar is Subtype) { ... }

is a matter of taste (the former is more efficient when compiled to JavaScript in the unsafe "trust type annotations" mode, should be completely equivalent on the VM).

In the future, we hope to improve on the cases that can cause type promotion. I'd hope for oldVar as Subtype; to work as a type-promoting "type assertion" (it's not new functionality, that statement is already known to throw if oldVar is not a Subtype, we just don't use that to promote the variable in code dominated by the check). There is no need for new syntax.

Crashing is not an option. The C++ crash is due to type unsoundness, and it doesn't have to crash, it can just do arbitrary memory corruption instead. The current Dart compiler does more checking than it needs to, because it still doesn't trust types completely (the legacy of Dart 1), but it should be possible to write a Dart compiler which makes assumptions about the memory layout of objects, without repeatedly checking that the object has the right type. Allowing an incorrectly typed object to flow into such code would be a security issue, and since we don't want that, it would preclude making such optimizations.

Hixie · 2019-03-05T02:40:35Z

We could also just ban overrides of runtimeType. Combine this with proposals elsewhere to add post-fix "fake methods" like .as, and define .runtimeType as one of these fake methods. Or combine it with the proposal to introduce sealed methods, and seal it. Or just special case runtimeType.

matanlurey · 2019-03-05T03:30:45Z

I personally really like the idea of making runtimeType an extension method on Object that is implemented in a private way.

…

On Mon, Mar 4, 2019, 6:40 PM Ian Hickson ***@***.***> wrote: We could also just ban overrides of runtimeType. Combine this with proposals elsewhere to add post-fix "fake methods" like .as, and define .runtimeType as one of these fake methods. Or combine it with the proposal to introduce sealed methods, and seal it. Or just special case runtimeType. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#35524 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKQ7mGLdDcc86xJFRY9ckHCsraTHzjwks5vTdkkgaJpZM4ZlETN> .

vsmenon added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Jan 2, 2019

Hixie mentioned this issue Feb 22, 2019

Less boilerplate for operator == #36004

Closed

a14n mentioned this issue Dec 2, 2019

implicit-casts:false in flutter/lib/src/rendering flutter/flutter#45720

Merged

9 tasks

lrhn added the closed-stale Closed as the issue or PR is assumed stale label Apr 9, 2025

lrhn closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

There's no good way to promote an argument #35524

There's no good way to promote an argument #35524

There's no good way to promote an argument #35524

There's no good way to promote an argument #35524

Comments