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

Inference cannot solve for T? == dynamic #1035

Open
lrhn opened this issue Jun 17, 2020 · 12 comments
Open

Inference cannot solve for T? == dynamic #1035

lrhn opened this issue Jun 17, 2020 · 12 comments
Labels
nnbd NNBD related issues

Comments

@lrhn
Copy link
Member

lrhn commented Jun 17, 2020

Example:

class C<T extends Object> {
  List<T?> foo;
  C(this.foo);
}
main() {
  C(<dynamic>[]);
}

This fails in nullsafety.dartpad.dev with the analyzer errors:

error
'dynamic' doesn't extend 'Object' - line 7
error
Couldn't infer type parameter 'T'. Tried to infer 'dynamic' for 'T' which doesn't work: Type parameter 'T' declared to extend 'Object'. The type 'dynamic' was inferred from: Parameter 'foo' declared as 'List<T?>' but argument is 'List<dynamic>'. Consider passing explicit type argument(s) to the generic. - line 7

Should we infer Object for T here? We do so for <Object?>[], and in practice dynamic and Object? are equivalent types wrt. subtyping, so choosing T as Object would allow a List<dynamic> as constructor argument.

(I hit the issue with an extension not applying to a dynamic list, even though it applies to a List<Object?>).

@lrhn lrhn added the nnbd NNBD related issues label Jun 17, 2020
@leafpetersen
Copy link
Member

leafpetersen commented Jun 18, 2020

This is a bug in the spec for constraint generation. We're missing the left top rule. Would you mind adding a language test for this if you get a chance?

@lrhn
Copy link
Member Author

lrhn commented Jun 18, 2020

dart-bot pushed a commit to dart-lang/sdk that referenced this issue Aug 24, 2020
See dart-lang/language#1035

Change-Id: Ieb258a020d8212f5488cc3d73978b8132223353d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/151760
Commit-Queue: Lasse R.H. Nielsen <lrn@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>
@leafpetersen
Copy link
Member

Spec landed, tests are out, implementation issues filed.

@leafpetersen
Copy link
Member

Fix to the fix landed above: #1200 (ordering was incorrect).

@leafpetersen
Copy link
Member

Implementation issues for reference:

@leafpetersen
Copy link
Member

Re-opening, since this is having difficulty landing.

@leafpetersen
Copy link
Member

Ok, this seems to have landed and stuck.

@leafpetersen
Copy link
Member

The following example seems to produce incorrect results still:

void main() {
  Future<Object?> bar1 = foo1(null); // Infers Object
  Future<dynamic> bar2 = foo1(null); // Infers Object?
  Future<Object?> bar3 = foo2(null); // Infers Object
  Future<dynamic> bar4 = foo2(null); // Infers Object
}


Future<T?> foo1<T extends Object?>(T x) {
  print('T is $T');
  return Future<T?>.value(null);
}

Future<T?> foo2<T extends Object>(T x) {
  print('T is $T');
  return Future<T?>.value(null);
}

I can't justify the results above based on the specification.

@scheglov @johnniwinther the CFE and analyzer agree on this, but I can't reconcile this with the spec, am I missing something?

@lrhn all of your tests use T extends Object instead of T extends Object? was there a reason for that? That seems to make the difference in the inference above.

cc @goderbauer

@lrhn
Copy link
Member Author

lrhn commented Oct 23, 2020

I think I was just testing the original issue, and wasn't aware that a bound of Object? was an issue too, I probaby assumed that solving for T? = dynamic with T bounded by Object? would just use T = dynamic (to preserve the dynamisim).

@chloestefantsova
Copy link
Contributor

@leafpetersen Apparently, it happens after the constraints are gathered. If my analysis is correct, the constraint gathering for bar2 ends up with dynamic constraining T, but then an additional step is taken: the bound is added to the set of constraints, and so two upper bounds dynamic and Object? are combined using DOWN to create one constrain Object?.

Here's the link of where that happens in the CFE:

https://github.com/dart-lang/sdk/blob/389201d6e40173a69276d2af4c1029dc7f354f13/pkg/front_end/lib/src/fasta/type_inference/type_schema_environment.dart#L535

@scheglov
Copy link
Contributor

I think #1200 was about dynamic <# T?, but here we have T? <# dynamic.

[methodInvocation][node: Future<dynamic> bar2 = foo1(null)]
[trySubtypeMatch: Future<T?> <# Future<dynamic>]
  [trySubtypeMatch: T? <# dynamic]
  [trySubtypeMatch: T <# dynamic]
  [trySubtypeMatch: Null <# dynamic]
constraints: {T extends Object?: _ <: T <: dynamic}
...skip
[_chooseTypeFromConstraints][downward inference]
  0 = {_TypeConstraint} 'T extends Object?' must extend 'dynamic'
  1 = {_TypeConstraint} 'T extends Object?' must extend 'Object?'
inferred: [Object?]

The analyzer has similar to CFE code that adds the extends clause to constraints.

@leafpetersen
Copy link
Member

@scheglov yes, you are correct, thanks. I was just looking at the rules and realizing the same thing - the tests and the spec update here only account for one direction.

leafpetersen added a commit that referenced this issue May 10, 2022
Fix issue from #1035 . Update the algorithm for constraint solving to prefer decomposing the `dynamic` or `void` types on the left into `Object?` when matching against `Q?` for some `Q`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nnbd NNBD related issues
Projects
None yet
Development

No branches or pull requests

4 participants