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

Analyser finds extension, but it wont compile #52077

Closed
FMorschel opened this issue Apr 18, 2023 · 17 comments
Closed

Analyser finds extension, but it wont compile #52077

FMorschel opened this issue Apr 18, 2023 · 17 comments
Labels
analyzer-spec Issues with the analyzer's implementation of the language spec area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@FMorschel
Copy link
Contributor

I have a file which looks like the following (you can test this error in dartpad.dev):

void main() {}

class Filter<T extends Filterable?, E extends Object?> {
  const Filter({
    required this.column,
    required this.filter,
  });

  final int column;
  final List<E> filter;

  bool test(T object) {
    if (object == null) return false;
    return filter.map(_constEmptyList).contains(
          object
              .when(_notEmptyList, otherwise: (_) => const [] as T)!
              .column(column),
        );
  }

  bool _notEmptyList(T self) {
    return (self is! List) || ((self as List).isNotEmpty);
  }

  R _constEmptyList<R>(R e) {
    return ((e is List) && e.isEmpty) ? const [] as R : e;
  }
}

mixin Filterable {
  T column<T extends Object?>(int index);
}

extension _When<T extends Filterable> on T {
  T? when(bool Function(T self) predicate, {T Function(T self)? otherwise}) {
    if (predicate(this)) return this;
    return otherwise?.call(this);
  }
}

It gives the following output:

Error compiling to JavaScript:
Info: Compiling with sound null safety.
lib/main.dart:16:16:
Error: The method 'when' isn't defined for the class 'Filterable'.
 - 'Filterable' is from 'package:dartpad_sample/main.dart' ('lib/main.dart').
              .when(_notEmptyList, otherwise: (_) => const [] as T)!
               ^^^^
Error: Compilation failed.

I'm not sure this is intended behaviour, but for me, it doesn't seem like that mostly because (as said in the title) Analyser tells me nothing is wrong.

@eernstg
Copy link
Member

eernstg commented Apr 18, 2023

It looks like a bug in the common front end (or perhaps the backend, but that seems less likely). Here is a simplified version of the example:

mixin Filterable {}

class Filter<T extends Filterable?> {
  void test(T object) {
    if (object is! Filterable) return; // Should promote `object` to `T & Filterable`.
    object.when(); // Accepted by analyzer. Error during compilation.
  }
}

extension _When<T extends Filterable> on T {
  T? when() => null;
}

void main() {}

This program causes the following compile-time error:

Error compiling to JavaScript:
Info: Compiling with sound null safety.
lib/main.dart:6:12:
Error: The method 'when' isn't defined for the class 'Filterable'.
 - 'Filterable' is from 'package:dartpad_sample/main.dart' ('lib/main.dart').
    object.when(); // CFE error.
           ^^^^
Error: Compilation failed.

I'll transfer this issue to the SDK repository because it seems to be a CFE bug.

@eernstg eernstg transferred this issue from dart-lang/language Apr 18, 2023
@eernstg eernstg added the legacy-area-front-end Legacy: Use area-dart-model instead. label Apr 18, 2023
@eernstg
Copy link
Member

eernstg commented Apr 18, 2023

@johnniwinther, do you agree that the extension _When should be applicable to a receiver of type T & Filterable?

@FMorschel
Copy link
Contributor Author

Originally, I had this extension with T extends Object but when I got the error I figured it had something to do with the fact that Filterable is a mixin, so I changed it. For everything else in my code, this extension for T extends Object works just fine.

@eernstg
Copy link
Member

eernstg commented Apr 18, 2023

Interesting! But the example I gave still causes a compile-time error with class Filterable {}.

@FMorschel
Copy link
Contributor Author

FMorschel commented Apr 18, 2023

Maybe it has something to do with dart-lang/sdk#52117?

@lrhn
Copy link
Member

lrhn commented Apr 18, 2023

It looks like the problem could be that while T & Filterable (which is NonNull(T extends Filterable?) is a subtype of Filterable, it's not a valid type argument to extension _When<X extends Fiterable>, because intersection types are not valid type arguments to begin with.

So, to invoke the extension, the compiler would have to choose either T or Filterable as the actual type argument, with only Filterable actually being valid. It likely chose T, because that's how intersection types are usually reduced to a single type.

Or maybe it does just look at T & Filterable and refuses to apply the extension, it's just very clever about giving an error message saying precisely which extension it is that it's not applying.
(But T & Filterable then prints as Filterable in the error message, which makes things very confusing, since Filterable itself clearly should work with the extension.)

@FMorschel
Copy link
Contributor Author

FMorschel commented Apr 18, 2023

It looks like the problem could be that while T & Filterable (which is NonNull(T extends Filterable?) is a subtype of Filterable, it's not a valid type argument to extension _When<X extends Fiterable>, because intersection types are not valid type arguments to begin with.

So, to invoke the extension, the compiler would have to choose either T or Filterable as the actual type argument, with only Filterable actually being valid. It likely chose T, because that's how intersection types are usually reduced to a single type.

Or maybe it does just look at T & Filterable and refuses to apply the extension, it's just very clever about giving an error message saying precisely which extension it is that it's not applying. (But T & Filterable then prints as Filterable in the error message, which makes things very confusing, since Filterable itself clearly should work with the extension.)

This explanation, at least for me, really seems to fit the dart-lang/sdk#52117 error.

@lrhn
Copy link
Member

lrhn commented Apr 19, 2023

I think the analyzer has issues too:

void main() {
  foo<List<int>?>([1]);
}

foo<T extends List<int>?>(T list) {
  if (list is List<int>) {
    // Promoted type T&List<int>
    List<int> l1 = list;
    T l2 = list;
    list.st<Exactly<T>>(); // When used to solve <T>, it become
    // list.st<Exactly<List<int>>>();
    list.foo;
    list.bar;
    list.baz;
    list.baz.st<Exactly<T>>(); // Unsound
  }
}

extension <S> on S {
  void st<X extends Exactly<S>>(){}
}

extension on List<int> {
  List<int> get foo {
    print("foo: List<int>");
    return this;
  }
}

extension <S> on S {
  S get bar {
    print("bar: $S");
    return this;
  }
}

extension <S extends List<int>> on S {
  S get baz {
    print("baz: $S");
    return this;
  }
}

typedef Exactly<T> = T Function(T);

(https://dartpad.dev/?id=f2bd890540648e55221aa5d6dbf8e469)

The lines:

    list.st<Exactly<T>>();
    // list.st<Exactly<List<int>>>();

show that the actual type argument passed to the extension is T, not List<int>. The second line is commented out because it's an error in both front-end and analyzer.

However, the front-end says that baz does not apply to list. One could argue that it should, but intersection types can make it not apply anyway, if it's not possible to choose the correct type argument.

The analyzer gives no error, and the line

    list.baz.st<Exactly<T>>(); // Unsound

shows (probably, always hard to be absolutely sure) that the type argument actually passed to baz is T.

That is an invalid type argument, since T is not a subtype of List<int>, it's not a valid type argument to the extension of baz, which has type parameters <S extends List<int>>.

So, analyzer is doing something very wrong here, so it's not obvious that it's the front-end which is being wrong.

@eernstg
Copy link
Member

eernstg commented Apr 19, 2023

Dart type inference is partially specified in inference.md, but it does not cover the situation where an intersection type is the static type of the receiver of an invocation which might call an extension method.

I've create #56028 in order to reach a conclusion about which rules we want. This issue may then be used to handle the implementation of the given approach, possibly with two subissues area-front-end and area-analyzer if needed.

@lrhn, I'd recommend that you create new issues if this comment shows unexpected behaviors in the analyzer that aren't covered by the intersection-type/extension-method topic which was the starting point for this issue.

@FMorschel
Copy link
Contributor Author

FMorschel commented Apr 19, 2023

I gave a look at inference.md, and I have another question, that I couldn't understand reading that file.

Why does when I do a code like the following, it gives me Object? even when it could give me num?

extension _When<R extends Object, T extends R> on T {
  R? when(R Function(T) fn) => null;
}

void main() {
  final number = 1; // int
  final result = number.when((_) => 1.2); //Object?
}

If this type of generics doesn't work, shouldn't it be warned with a lint?

@eernstg
Copy link
Member

eernstg commented Apr 20, 2023

It is rarely useful for an extension to declare a type parameter whose value is unconstrained by the on type of the extension (in this example: R), because that type variable will always be inferred to have the value which is the declared bound.

The reason for this is that all actual type arguments for the extension are inferred (unless provided explicitly) based on the static type of the receiver, with no input from any other source. In particular, the context type is _. This means that a type variable like R has no constraints other than the bound, and it will then be inferred as that bound.

However, the example doesn't provide any justification for the relationship T extends R. Why don't you just do this?:

extension _When<T> on T {
  R? when<R extends Object>(R Function(T) fn) => null;
}

If you do need the relation T extends R then I'm afraid you will need a lower bound, which is a feature that we do not (yet) have in the language:

extension _When<T> on T {
  R? when<R extends Object super T>(R Function(T) fn) => null;
}

We could also consider an approach where you handle the same computation using a regular function. This means that the receiver will now be the first argument, and remaining arguments are pushed back to position 2, 3, etc. The point is that this makes the type arguments dealing with the "receiver" and the type arguments dealing with the arguments available at the same time, hence allowing us to specify a larger set of relationships:

R? when<R extends Object, T extends R>(T self, R Function(T) fn) => null;  

void main() {
  final number = 1; // int
  final result2 = when(number, (_) => 1.2); // Error, inference failed. `<num>` would work.
}

There is a special difficulty here, because type inference would need to process the actual arguments of the invocation as a group in order to know that it needs to use num as the actual type argument.

The current approach causes the arguments to be split into phases (because the first argument has type T and the second argument is a function literal whose parameter type is also T, so the type of fn depends on the type of self), and the phasing allows those dependencies to be taken into account. This was a very significant improvement when it was added in 2.18.

However, in this particular case it isn't sufficient, because there is a need to flow information in the other direction: returning 1.5 implies that double <: X must hold, so we can't use X == int based on the argument passed to self alone.

See dart-lang/language#3013 where this difficulty is reported.

@FMorschel
Copy link
Contributor Author

FMorschel commented Apr 20, 2023

Thank you for your explanation. Just so you understand better why I was asking this.

I have as you've seen, this extension:

extension When<T extends Object> on T {
  T? when(bool Function(T self) predicate, {T Function(T self)? otherwise}) {
    if (predicate(this)) return this;
    return otherwise?.call(this);
  }
}

This is used by me mostly inside Flutter projects since everything is mostly a one-line code, I sometimes find myself having to test for something (even more useful when is some nested value) so I actually use this extension for those cases.

But, some days ago I tried to use this extension to set a Widget on my screen, since it has only one type parameter it was telling me the other widget wasn't of the same type T. To solve that I've done the following:

extension WhenWidget<T extends Widget> on T {
  T? when(bool Function(T self) predicate, {T Function(T self)? otherwise}) {
    if (predicate(this)) return this;
    return otherwise?.call(this);
  }
}

This solves my case because is more specific so it infers that it should be used. Only asked because if I end up having to do more of these extensions, I will have a lot of work having to create one more for every more specific type.

@lrhn lrhn added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed legacy-area-front-end Legacy: Use area-dart-model instead. labels Apr 20, 2023
@lrhn
Copy link
Member

lrhn commented Apr 20, 2023

As commented in #56028, this is an analyzer bug, as things are currently specified (at least based on the extension method feature specification, haven't checked the language spec integration of that).

The type inference applied to object.when should behave the same as other inferences where a type argument is found from an intersection type, which means falling back to the type variable.
That type variable is not a valid type argument to the extension, which means the extension is not applicable, and therefore the front-end behavior of saying "no when method declared" is correct.

(The error message is confusing, it should say, Error: The method 'when' isn't defined for the type 'T & Filterable'. Bonus points for saying that the extension WhenWidget does not apply, because T is not a valid type argument to it.)

@srawlins srawlins added P3 A lower priority bug or feature request analyzer-spec Issues with the analyzer's implementation of the language spec labels Apr 24, 2023
@FMorschel
Copy link
Contributor Author

FMorschel commented Jul 11, 2023

Hi again! I'm not entirely sure this is the same thing, but the last line with content here:

formatter = widget.formatter ?? _formatter; //A value of type 'TextInputFormatter' can't be assigned to a variable of type 'NumberFormatter<T>'.

it is showing this error. Here is the complete code for it.
I don't believe this should be warning but please let me know if you are able to figure this out.

class CustomDoubleField<T extends double?> extends StatefulWidget {
  const CustomDoubleField({
    this.formatter,
    super.key,
  });

  final NumberFormatter<T>? formatter;

  @override
  State<CustomDoubleField<T>> createState() => _CustomDoubleFieldState<T>();
}


class _CustomDoubleFieldState<T extends double?> extends State<CustomDoubleField<T>> {
  static final _formatter = NumberFormatterDecimal<double>(
    maxDecimalDigits: 3,
    minDecimalDigits: 0,
    decimalSeparator: ',',
    groupSeparator: '.',
  );

  late NumberFormatter<T> formatted;

  @override
  void initState() {
    super.initState();
    formatter = widget.formatter ?? _formatter; //A value of type 'TextInputFormatter' can't be assigned to a variable of type 'NumberFormatter<T>'.
  }

  //...
}

Even though NumberFormatterDecimal is:

class NumberFormatterDecimal<T extends num?>
    with EquatableMixin
    implements NumberFormatter<T> {
  //...
}

abstract class NumberFormatter<T extends num?> implements TextInputFormatter {
  T parse(String txt);
  String format(T numero);
}

@lrhn
Copy link
Member

lrhn commented Jul 11, 2023

It's not the same. This code is just not type-sound.

(I assume late NumberFormatter<T> formatted; should be late NumberFormatter<T> formatter;, or the assignement should be formatted = ..., but I'm guessing the former.)

I can derive why you get the type that you do, but the bigger underlying problem is that _formatter is not assignable to _CustomDoubleFieldState.formatter.
The type NumberFormatterDecimal<double> (or NumerFormatter<double> for that matter) is not a subtype of NumberFormatter<T>.
Not in general. It might be for some T values, but not if T is, say, Never, and the code has to be valid for all types bound to T, otherwise the compiler will complain.

The problem is the T extends double. Why is that there, when double doesn't have any useful subtypes?

(And the reason you get TextInputFormatter is:
The static type of widget.formatter is that of CustomDoubleFIeld<T>.formatter, which is NumberFormatter?. If that's null, we go to the second operand of ??, and the static type of _formatterisNumberFormatterDecimal. Then we try to find a common superinterface of NumberFormatterandNumberFormatterDecimal`, using the UP ("least-like upper bound") algorithm.

The implemented superinterfaces of those two are:

  • .formatter: NumberFormatter<T>, TextInputFormatter, Object
  • _formatter: NumberFormatterDecimal<double>, NumberFormatter<double>, TextInputFormatter, Object.

The least interface those two have in common is TextInputFormatter, which is better than the other alternative Object, so thats the static type of the ??-expression.

It's then not assignable to the context type NumberFormatter<T>, which is correct.

Even with improved context-type guidance, which I don't know if we have added yet or just talked about, where we'd just give NumeberFormatter<T> back if both types are subtypes that and the UP result isn't, that won't work here, because NumberFormatterDecimal<double> was not a subtype of NumberFormatter<T>.)

@FMorschel
Copy link
Contributor Author

I see. Thank you a lot for the explanation!

@srawlins
Copy link
Member

srawlins commented Oct 9, 2023

Closing this in favor of #53711. This discussion took many tangents and was hard for me to follow to see what our AI is. I think I've summarized and quoted the important parts over there.

@srawlins srawlins closed this as completed Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-spec Issues with the analyzer's implementation of the language spec area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. 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

4 participants