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 aren't considered with generic-method types #26965

Closed
nex3 opened this issue Jul 25, 2016 · 6 comments
Closed

"is" checks aren't considered with generic-method types #26965

nex3 opened this issue Jul 25, 2016 · 6 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@nex3
Copy link
Member

nex3 commented Jul 25, 2016

If I run the analyzer over the following code in strong mode:

void fn/*<T>*/(/*=T*/ object) {
  if (object is String) print(object.substring(1));
}

I get the error "The method 'substring' is not defined for the class 'Object'.". This seems incorrect, since I'm asserting that object is String immediately before calling substring().

@nex3 nex3 added analyzer-strong-mode type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jul 25, 2016
@jmesserly
Copy link

I think this is covered by one of the items at:
#25565
... but that said, this one is probably fairly easy to fix on its own -- we just need to consider the type parameter bound; we already made several fixes along those lines for Strong Mode.

@munificent @leafpetersen - any word lately on improved type promotion and/or related features? we're in a really tough spot right now, with so much stuff not working. I know I bump into it quite a lot too.

@jmesserly jmesserly added P2 A bug or feature request we're likely to work on area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Jul 26, 2016
@leafpetersen
Copy link
Member

I don't think considering the bound helps here, at least not directly. Currently promotion is only done if the type you're testing against is a subtype of the static type of the thing you're testing, which isn't the case here (and in general will never be the case for type variables without contra-variant bounds except in trivial cases). There does seem to a valid argument for not-promoting in some cases, but I can't think of any offhand in the type variable case. It seems like promoting if the type being tested against is a subtype of the bound would make sense.

@jmesserly
Copy link

jmesserly commented Jul 26, 2016

It seems like promoting if the type being tested against is a subtype of the bound would make sense.

yeah, agreed. I had the same thought at first: "well don't promote because T could be anything". But given a type parameter T extends S, we will only allow you to do operations on S statically. So given: (T t) { if (t is R) { ... } }, where R <: S, it would be safe to promote t to R, as that is a superset of the operations you could express before promotion. (edit: clarity)

@jmesserly jmesserly self-assigned this Aug 4, 2016
@jmesserly
Copy link

@leafpetersen
Copy link
Member

It occurs to me that an even better change would be to allow refinement of the upper bound, so that within the scope of the check the variable is treated as having an upper bound of the refined type. I don't think that fits into the existing type promotion structure, but something to think about as a future refinement.

@jmesserly
Copy link

fyi -- I filed #27040 to track your comment Leaf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants