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

Strong mode override inference not checked against initializer #25546

Closed
vsmenon opened this issue Jan 20, 2016 · 5 comments
Closed

Strong mode override inference not checked against initializer #25546

vsmenon opened this issue Jan 20, 2016 · 5 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@vsmenon
Copy link
Member

vsmenon commented Jan 20, 2016

This code should show an error in strong mode - B's map breaks A's contract.

abstract class A {
  Map<int, int> get map;
}

class B extends A {
  var map = { 42: "world" };
}
@vsmenon vsmenon added Priority-Medium area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. labels Jan 20, 2016
@jmesserly
Copy link

also, if the RHS was correctly typed, it should push the type down via inference.

@munificent munificent mentioned this issue Feb 18, 2016
35 tasks
@jmesserly jmesserly added Type-Defect P1 A high priority bug; for example, a single project is unusable or has many test failures and removed Priority-Medium labels Feb 18, 2016
@kevmoo kevmoo added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed Type-Defect labels Mar 1, 2016
@bwilkerson
Copy link
Member

I'm not sure what the correct behavior is. Do we want to set the type of B.map to the type inferred for the literal (Map<int, String>) and notify the user that this is not compatible with the overridden getter, or do we want to set the type of B.map to the same type as A.map and notify the user that the initializer cannot be assigned to the field? If the former, what if the initializer is of the form {}?

@bwilkerson
Copy link
Member

@vsmenon @leafpetersen Thoughts?

@leafpetersen
Copy link
Member

I believe that this is almost (but perhaps not entirely) stale. I cannot reproduce this exactly in bleeding edge (nor the most recent dev release).

What I would expect to happen is the following:

  1. Infer instance members would fill in the type for B.map as Map<int, int>
  2. Downwards inference would push this type down into the map literal, making it Map<int, int>
  3. We'd get an error indicating that a string value can't be used in a Map<int, int>

Step 2 is not happening. For this example, it doesn't really matter, because upwards inference fills in a type for the literal, and then we reject the program because the two types don't match. However, for this modified example it matters:

abstract class A {
  Map<int, List<int>> get map;
}

num n;
class B extends A {
  var map = { 42: [] };
}

class C extends A {
  get map => { 43: [] };
}

If I print out the types inferred for everything, it looks like C.map is correctly pushing the inferred type down into the literal, filling in the types as <int, List<int> on the map literal, and <int> on the empty list literal. But for B.map, it looks like <int, List<dynamic>> and <dynamic> are getting filled in. This is bad, since this will fail at runtime.

Poking around at this basically got me to the point where the fix looks obvious, so I'll claim this.

@leafpetersen leafpetersen self-assigned this Apr 22, 2016
@leafpetersen
Copy link
Member

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. P1 A high priority bug; for example, a single project is unusable or has many test failures type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

5 participants