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

No type inference from is! #35019

Closed
jamesderlin opened this issue Nov 1, 2018 · 6 comments
Closed

No type inference from is! #35019

jamesderlin opened this issue Nov 1, 2018 · 6 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-duplicate Closed in favor of an existing report

Comments

@jamesderlin
Copy link
Contributor

  • Dart SDK 2.0.0
  • Linux, also tested with dartpad

The following code is accepted without error:

class Base {
}

class Derived extends Base {
  void foo() {
  }
}

void main() {
  Base d = Derived();
  if (d is Derived) {
    d.foo();
  }
}

However, if I invert the d is Derived check:

  if (d is! Derived) {
  } else {
    d.foo();
  }

then I get an error:

Error: The method 'foo' isn't defined for the class '#lib1::Base'.

(The same thing happens if I try !(d is Derived).)

This seems inconsistent. Couldn't we internally invert is! and the if-else clauses to achieve the same behavior as the first version?

(And if anyone is wondering why I don't just always use the first version, there are cases where one path has much more code than the other, and for readability I prefer to have the simpler block first, e.g.:

if (d is! Derived) {
  // assert(false); // Or run some other failure handler
} else {
  d.foo();
  // Followed by a lot more code.
}

Of course, in that particular case, it be even nicer if:

assert(d is Derived);
d.foo();

worked.)

@matanlurey matanlurey added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Nov 1, 2018
@matanlurey
Copy link
Contributor

This looks like it is ~mostly covered by @munificent's #29624, which has stalled for quite some time. @mit-mit does it make sense to try and file some tracking issue(s) in dart-lang/language for this (and related) cases?

@jamesderlin:

assert(d is Derived);

An assert statement cannot be used to change the code, as it is only executed during development mode(s). This means your program would work different (and promote type differently) in release and development mode, which sounds quite bad.

@lrhn
Copy link
Member

lrhn commented Nov 2, 2018

This is a thing that we are aware of and want to address at some point in the future. If we introduce non-nullable types, we will need non-null promotion as well, so revisiting type promotion at that point seems reasonable.

As Matan says, you can't use assert to guarantee anything since it's not always executed.
An alternative could be x as Derived; which would always throw if x is not a Derived (or null, but let's assume a non-nullable type here).

@lrhn lrhn closed this as completed Nov 2, 2018
@lrhn lrhn added the closed-duplicate Closed in favor of an existing report label Nov 2, 2018
@bwilkerson
Copy link
Member

@lrhn Is there a language issue tracking the need for better type promotion?

@munificent
Copy link
Member

If we introduce non-nullable types, we will need non-null promotion as well, so revisiting type promotion at that point seems reasonable.

Better type promotion is useful even without non-nullable types, so I think it's good to keep in mind that we can do earlier if we want.

@jamesderlin
Copy link
Contributor Author

jamesderlin commented Nov 2, 2018

An assert statement cannot be used to change the code, as it is only executed during development mode(s). This means your program would work different (and promote type differently) in release and development mode, which sounds quite bad.

I realize that assert is not executed in release builds. However, conceivably its expression could still be used for type inference at compilation time. (I also presume that it's a lot of work and probably isn't worth doing.)

@munificent
Copy link
Member

Moving this over to: dart-lang/language#80

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). closed-duplicate Closed in favor of an existing report
Projects
None yet
Development

No branches or pull requests

5 participants