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

is-checks should not throw #28988

Closed
floitschG opened this issue Mar 8, 2017 · 7 comments

Comments

@floitschG
Copy link
Contributor

commented Mar 8, 2017

The following program leads to a runtime exception in DDC:

typedef int F();
typedef String G();

Object foo() => () { return 499; };

main() {
  Object f = foo();
  print(f is F);
  print(f is G);
}
true

.../dart_sdk.js:2540
  throw new _js_helper.StrongModeErrorImplementation(message);
  ^
Strong mode is-check failure: () -> int does not soundly subtype G(() -> String)

@floitschG floitschG changed the title is-checks should not fail is-checks should not throw Mar 8, 2017

@leafpetersen

This comment has been minimized.

Copy link
Member

commented Mar 8, 2017

This is basically working as intended. In order to preserve semantic coherence with non-strong mode platforms, we throw an exception at runtime if you ask an "is" check that we can't guarantee will produce the same result on all platforms. It's based on a heuristic, so we could in principle do better, but I'm not sure it's a priority for us right now.

@floitschG

This comment has been minimized.

Copy link
Contributor Author

commented Mar 8, 2017

Sounds like a good idea. However, the error message should be completely different then.

@vsmenon

This comment has been minimized.

Copy link
Member

commented Mar 31, 2017

Filed #29216 for the error message. Let's keep this one for the throw.

@vsmenon

This comment has been minimized.

Copy link
Member

commented Mar 31, 2017

As long as we have this throw, we'll need to avoid code like this:

https://github.com/dart-lang/sdk/blob/master/sdk/lib/async/future_impl.dart#L459

@floitschG @lrhn @leafpetersen - thoughts? DDC could start just returning the sound result (instead of throwing if the answer differs from Dart 1). There is the semantic coherence issue Leaf mentions above though.

@lrhn

This comment has been minimized.

Copy link
Member

commented Apr 2, 2017

I would suggest starting to migrating DDC to Dart 2 semantics now.

I know there are cases where code will do different things in the presence of dynamic, but you already do that with List<dynamic> not being assignable to List<int>, so I don't see x is List<T> being a bigger difference just because it happens at runtime.

With FutureOr, we actually need to do the is Future<T>, not just is Future, to detect the correct Future branch of, e.g., FutureOr<Future<Object>>.
Not that I expect people to actually write that, but it's valid code and we should act correctly on it.

@jmesserly

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2017

this one became more urgent, because we need to make sure the migrated Dart 2 tests can use these is checks without throwing.

@jmesserly jmesserly added the P1-High label Aug 4, 2017

@jmesserly

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2017

@jmesserly jmesserly closed this in 65e6095 Aug 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.