Skip to content

Every subexpression's type should be checked for assignability to the greatest closure of its context #31792

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

Open
stereotype441 opened this issue Jan 8, 2018 · 5 comments
Assignees
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. P2 A bug or feature request we're likely to work on

Comments

@stereotype441
Copy link
Member

In #31776 (comment), @leafpetersen says that the intention of the local type inference proposal is that in general, subexpressions should be checked for assignability to the greatest closure of the subexpression's context. If the assignability criterion is met because the subexpression's type is a subtype of the greatest closure of the context, then no further action needs to be taken. If not, then either an implicit downcast needs to be inserted, or an error should be reported.

This is slightly different from what was implemented in the front end. The front end only checks assignability (and only inserts implicit downcasts) where necessary for soundness. In most cases, there is no observable difference, since usually the contexts used in type inference are precisely those that are necessary for soundness. However, there are a few exceptions, for example:

  Object o;
  int x = o..toString(); // inferred as `(o..toString()) as int`; should be (o as int)..toString()
  int y = o == x ? o : x; // inferred as (o == x ? o : x) as int; should be o == x ? o as int : x
@lrhn
Copy link
Member

lrhn commented Jan 8, 2018

So, more importantly, the following would be valid:

Object o;
int x = o..toDouble();

Correct ?

@stereotype441
Copy link
Member Author

@lrhn Yes. Your example is currently rejected by the front end and the analyzer, and as I understand it under Leaf's proposed interpretation, it would be interpreted as int x = (o as int)..toDouble();, which is valid.

@leafpetersen
Copy link
Member

Yes, that's the intention. We actually had an issue filed about this from a user who was surprised not to get good completions from code exactly like the above, and I think it's the right thing to do.

@jensjoha jensjoha added the P2 A bug or feature request we're likely to work on label Jan 9, 2018
@bwilkerson
Copy link
Member

@danrubel It would be good for us to capture this as a test case for code completion.

dart-bot pushed a commit that referenced this issue Mar 7, 2018
This CL changes the handling of writeContext (for complex
assignments), returnOrYieldContext (for closures), expectedType (for
ensureAssignable), and receiverType (for findInterfaceMember) so that
they no longer use `null` to represent a context; they use UnknownType
or DynamicType as appropriate.

It also changes the handling of expectedType (for ensureAssignable) so
that it does not require the type to be known.  That makes this CL
simpler and paves the way for fixing #31792.

Change-Id: Ib1be06182f9a9e9f77a9eb81f5daf2364de0f3a7
Reviewed-on: https://dart-review.googlesource.com/44800
Commit-Queue: Paul Berry <paulberry@google.com>
Reviewed-by: Kevin Millikin <kmillikin@google.com>
@stereotype441
Copy link
Member Author

It turns out that I don't have the cycles to work on this right now, so I'm unassigning. FE team, if you want to discuss implementation strategies, please feel free to drop me a line and we can set up a VC.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. P2 A bug or feature request we're likely to work on
Projects
None yet
Development

No branches or pull requests

5 participants