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

Unexpected generic typing when using ternary operator and casting to base type #53663

Closed
idraper opened this issue Sep 30, 2023 · 2 comments
Closed
Labels
closed-duplicate Closed in favor of an existing report

Comments

@idraper
Copy link

idraper commented Sep 30, 2023

Dart SDK version: 3.1.2 (stable) on "windows_x64"

I think I understand why the contrived example below (but I ran into it with my actual project) is throwing an error, but I'm having trouble understanding why the analyzer cannot determine the correct type. I've also run into some general oddities regarding this case that I'd love some clarification on.

abstract class Base<T> {
  factory Base(int val) => BaseImpl1(val == 0) as Base<T>;
  T doSomething(T input);
}

class BaseImpl1 implements Base<bool> {
  BaseImpl1(this.someVal);
  bool someVal;
  @override
  bool doSomething(bool input) => someVal = input;
}

class BaseImpl2 implements Base<int> {
  BaseImpl2();
  @override
  int doSomething(int input) => 0;
}

void main() {
  final someCondition = true;
  Base obj = someCondition ? BaseImpl1(false) : BaseImpl2(); // this throws an error
  late Base obj2;
  if (someCondition) {
    obj2 = BaseImpl1(false); // doesn't throw
  } else {
    obj2 = BaseImpl2(); // doesn't throw
  }
}

My understanding is that since BaseImpl1 implements the specific generic <bool> and BaseImpl2 implements the specific generic <int>, the two ternary conditions cannot be implicitly cast to return the generic Base<dynamic> like I want. What doesn't make sense is why the same behavior doesn't happen with the if statement (it doesn't throw). I would expect the ternary to be able to resolve the types the same way as the conditional flow.

So... I guess this is happening because Dart doesn't allow casting generics (like List<int> -> List<bool> isn't allowed) but clearly there is some support since it is fine for the conditional statement. Is something else going on here that I'm not aware of? Is it okay with the conditional because it is casting to dynamic? but if so, why is it not the same with the ternary operator?

Additionally, it is odd to me that replacing the ternary operator with:

  Base obj = someCondition ? BaseImpl1(false) : (BaseImpl2() as Base); // no longer throws

no longer throws. This doesn't make sense to me because while I've explicitly cast BaseImpl2(), I haven't cast BaseImpl1(false) so I'd expect the analyzer to still complain about assigning Object to Base since it is trying to resolve types Base<bool> and Base<dynamic>.

Another (probably separate?) oddity I don't understand is that the analyzer doesn't show any problems with the following:

  final obj = someCondition ? BaseImpl1(false) as Base : BaseImpl2(); // shows no linter problems (except dead code from contrived example)

But if the as statement is wrapped in parenthesis, it says it is an unnecessary cast (which it isn't according to the analyzer, since it throws without it).

  final obj = someCondition ? (BaseImpl1(false) as Base) : BaseImpl2(); // shows `Unnecessary cast.  Try removing the cast.`

And lastly, as a side note:

While this isn't critical since I can cast the objects myself in this case, I found I could solve the issue by changing implementation classes to implement the base class with the generic type dynamic and using covariant to allow override of the base class functionality. In my case where I ran into this, this actually doesn't work for me since I am actually calling a static function that returns a more specific implementation class and I don't want to lose the typing information :/ (so I just use as to cast it as described above).

abstract class Base<T> {
  factory Base(int val) => BaseImpl1(val == 0) as Base<T>;
  T doSomething(covariant T input); // note the covariant
}

class BaseImpl1 implements Base { // no generic - implements Base<dynamic>
  // same as above
}

class BaseImpl2 implements Base { // no generic - implements Base<dynamic>
  // same as above
}

void main() {
  final someCondition = true;
  Base obj = someCondition ? BaseImpl1(false) : BaseImpl2(); // no longer throws
}

@eernstg
Copy link
Member

eernstg commented Sep 30, 2023

It is all working as intended, but not quite as desired. ;-)

You might wish to take a look at this topic: https://github.com/dart-lang/language/issues?q=is%3Aopen+is%3Aissue+label%3Aleast-upper-bound, where it is discussed and explained in great detail what is going on in situations like this one.

In particular, you could take a look at this issue, and perhaps vote for it: dart-lang/language#1618.

First, the underlying problem: An expression needs to have a static type, and in particular an expression of the form b ? e1 : e2 will have a specific static type based on the static types of e1 and e2.

In order to make this sound we'll have to choose a type which is a supertype of the type of e1 and a supertype of the type of e2, that is, an upper bound. One such type is a top type like Object? or dynamic (this will always work, but it drops far too much information). So we'd like to find the least upper bound. If we just go ahead and do that using the obvious algorithm then we get into an infinite loop in some cases. This problem was being discussed in connection to Java at the time where this algorithm was designed for Dart, so Dart was designed to use an approach that avoids recursive calls in the least upper bound algorithm. In return for that safety we get too general types now and then. In particular, we get Object as the least upper bound of BaseImpl1 and BaseImpl2.

(OK, it's not a "least" upper bound, because we can't in general find the least one, but it is a "rather low" upper bound, so we call it the standard upper bound.)

The proposal in dart-lang/language#1618 says that we should type check the ?: expressions ('conditional expressions') using the context type, that is, the type of result which is expected by the surrounding code. If we do that then we'll simply check (in your example) that BaseImpl1 is a subtype of Base<dynamic>, and BaseImpl2 is also a subtype of Base<dynamic> and hence we can give the entire conditional expression the type Base<dynamic>.

So if we adopt dart-lang/language#1648 then your example just works, done!

However, right now you can do what you already mentioned:

// ignore_for_file: unused_local_variable

abstract class Base<T> {
  factory Base(int val) => BaseImpl1(val == 0) as Base<T>;
  T doSomething(T input);
}

class BaseImpl1 implements Base<bool> {
  BaseImpl1(this.someVal);
  bool someVal;
  @override
  bool doSomething(bool input) => someVal = input;
}

class BaseImpl2 implements Base<int> {
  BaseImpl2();
  @override
  int doSomething(int input) => 0;
}

var someCondition = true;

void main() {
  Base obj = someCondition
      ? (BaseImpl1(false) as dynamic) as Base<Object>
      : BaseImpl2(); // this throws an error
  late Base obj2;
  if (someCondition) {
    obj2 = BaseImpl1(false); // doesn't throw
  } else {
    obj2 = BaseImpl2(); // doesn't throw
  }
}

(I disabled a lint and moved the declaration of someCondition to silence some other lints).

The point is that you stop asking for the standard upper bound of BaseImpl1 and BaseImpl2 and start asking for the standard upper bound of Base<Object> (which is the most precise type you can get, and safer than Base<dynamic> in the following code), and that's easy: When one operand is a subtype of the other then the standard upper bound is that other type.

In short, we'd like to have a better standard upper bound computation and also to introduce the context as a source of information, but right now you should probably just be prepared to give the standard upper bound algorithm a bit of help now and then. ;-)

@lrhn lrhn closed this as completed Oct 1, 2023
@lrhn lrhn added the closed-duplicate Closed in favor of an existing report label Oct 1, 2023
@idraper
Copy link
Author

idraper commented Oct 4, 2023

Thanks for the detailed response as always @eernstg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-duplicate Closed in favor of an existing report
Projects
None yet
Development

No branches or pull requests

3 participants