Skip to content

Type inference is dependent on the order that type parameters are defined #40423

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
leonsenft opened this issue Jan 31, 2020 · 9 comments
Open
Labels
area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec legacy-area-front-end Legacy: Use area-dart-model instead. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@leonsenft
Copy link

leonsenft commented Jan 31, 2020

Versions:

  • DartPad on SDK 2.7.0
  • VM/dart2js/DDC on 2.8.292505767-edge+google3-v2 (google3)

Here's a working reproduction https://dartpad.dev/a8a92c56149499dd479488f1cca5bff9.

class Value {}

class Renderer<T> {}

// When V is defined before R, type inference works in all cases.
class WorkingOrder<V extends Value, R extends Renderer<V>> {}

// When V is defined after R, type inference fails in some cases.
class BrokenOrder<R extends Renderer<V>, V extends Value> {}

void main() {
  // Correctly infers type.
  var working1 = WorkingOrder();
  print(working1.runtimeType); // WorkingOrder<Value, Renderer<Value>>.

  // Correctly infers type arguments to bounds.
  WorkingOrder working2 = WorkingOrder();
  print(working2.runtimeType); // WorkingOrder<Value, Renderer<Value>>.

  // ERROR: Incorrectly infers BrokenOrder<Renderer<dynamic>, Value>.
  var broken = BrokenOrder();

  // Correctly infers type arguments to bounds.
  BrokenOrder working3 = BrokenOrder();
  print(working3.runtimeType); // BrokenOrder<Renderer<Value>, Value>.
}
  • Inference behaves differently when inferring the type of a variable declared as var versus the raw type - this is unexpected.
  • The inference algorithm used for inferring the type of a variable declared as var appears to be dependent on the order in which type parameters are declared on the resulting type, when one of those type parameters depends on the other.
@leonsenft
Copy link
Author

@leafpetersen @vsmenon @Markzipan @nshahan this is the inference issue I was showing you all yesterday.

@a-siva a-siva added the area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. label Jan 31, 2020
@leonsenft
Copy link
Author

@a-siva Sorry if I wasn't clear enough, this isn't restricted to the web. If you run my reproduction on the VM, the same error occurs.

$ dart main.dart

main.dart:19:17: Error: Inferred type argument 'Renderer<dynamic>' doesn't conform to the bound 'Renderer<V>' of the type variable 'R' on 'BrokenOrder'.
 - 'Renderer' is from 'main.dart'.
Try specifying type arguments explicitly so that they conform to the bounds.
  var broken1 = BrokenOrder();
                ^
main.dart:9:19: Context: This is the type variable whose bound isn't conformed to.
class BrokenOrder<R extends Renderer<V>, V extends Value> {}

@leonsenft leonsenft removed the area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. label Jan 31, 2020
@a-siva a-siva added the legacy-area-front-end Legacy: Use area-dart-model instead. label Jan 31, 2020
@leonsenft
Copy link
Author

The error also appears in the analyzer, which I didn't think was using the front end, but that would imply both the CFE and analyzer have the exact same bug.

@johnniwinther johnniwinther added the legacy-area-analyzer Use area-devexp instead. label Jan 31, 2020
@johnniwinther
Copy link
Member

cc @stereotype441 @stefantsov

@srawlins srawlins added the dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec label Jun 16, 2020
@srawlins srawlins added the P3 A lower priority bug or feature request label Jan 28, 2021
@srawlins srawlins added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Nov 18, 2021
@srawlins srawlins added the contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) label May 20, 2022
@srawlins
Copy link
Member

srawlins commented Dec 6, 2024

@eernstg Is this the issue that was fixed recently with dart-lang/language#3009 ? I haven't looked at enabling that experiment personally...

@leonsenft
Copy link
Author

As of Dart SDK 3.7.0-183.0.dev, this is still broken from a quick test in DartPad.

@srawlins
Copy link
Member

srawlins commented Dec 9, 2024

Yeah I don't think dartpad has that experiment enabled. Even on the main branch.

@eernstg
Copy link
Member

eernstg commented Dec 9, 2024

This does look like the kind of example that shows how #3009 and '--enable-experiment=inference-using-bounds' gives rise to improved type inference. The feature is enabled by default in Dart 3.7, which means that it is available in the DartPad main channel, but not beta or stable.

However, the example does not match precisely. First, in the two cases where the variable type is declared as a raw type (that is, without actual type arguments), the inference process is controlled by the declared type, and the declared type is obtained by instantiation to bound (not inference), which forces the value of the type argument V to be the bound Value, and R to be Renderer<Value>.

In these cases the ordering of the type variables is irrelevant because the outcome of the inference step is done by "remote control" by specifying the variables to have the types WorkingOrder<Value, Renderer<Value>> and BrokenOrder<Renderer<Value>, Value> (it's spelled WorkingOrder and BrokenOrder, but that's what it means).

Here is the remaining example program, renamed to put the emphasis on the order:

class Value {}

class MyValue implements Value {}

class Renderer<T> {}

class BeforeBoundOrder<V extends Value, R extends Renderer<V>> {
  final V value;
  BeforeBoundOrder(this.value);
}

class AfterBoundOrder<R extends Renderer<V>, V extends Value> {
  final V value;
  AfterBoundOrder(this.value);
}

void main() {
  var v1 = BeforeBoundOrder(MyValue());
  print(v1.runtimeType); // 'BeforeBoundOrder<MyValue, Renderer<MyValue>>'.

  // ERROR: Tried to infer 'Renderer<Object?>' for 'R' which doesn't work: ...
  var v2 = AfterBoundOrder(MyValue());
}

The failure occurs even when 'inference-using-bounds' is enabled, because we're computing R at a time where the bound of V hasn't yet been taken into account, and then we get Renderer<Object?> which isn't a subtype of Renderer<MyValue>.

The change to type inference which will eliminate the ordering dependency hasn't been implemented yet (and some discussions about how to do it may still be needed). @chloestefantsova, perhaps you can say more about this?

The example shows another thing, though. If the error line is commented out then the program runs and prints BeforeBoundOrder<MyValue, Renderer<MyValue>>. This shows that the value of V is MyValue rather than Value which will yield a more tight typing of various things (in particular the new object has a more precise list of type arguments, but it also makes the type of the variable more precise, which could affect code below). This is an improvement which has been brought about by #3009 / inference-using-bounds.

@srawlins
Copy link
Member

Thanks for the investigation, @eernstg !

@bwilkerson bwilkerson added area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. contributions-welcome Contributions welcome to help resolve this (the resolution is expected to be clear from the issue) dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec legacy-area-front-end Legacy: Use area-dart-model instead. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

6 participants