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

Ternary operator can not infer return type FutureOr #2432

Closed
vgtle opened this issue Aug 24, 2022 · 10 comments
Closed

Ternary operator can not infer return type FutureOr #2432

vgtle opened this issue Aug 24, 2022 · 10 comments
Labels
request Requests to resolve a particular developer problem

Comments

@vgtle
Copy link

vgtle commented Aug 24, 2022

The Problem

Returning a value of type FutureOr<T> using a ternary operator where one value is of type T while the other is of type Future<T> will result in the error A value of type 'Object' can't be returned from the method <method-name> because it has a return type of 'FutureOr<T>'. and can not be compiled. See the code sample below for an example.

To resolve this issue the value of type T has to be casted as FutureOr<T> (E.g. "test" as FutureOr<String>).

Code Sample

import 'dart:async';

void main() {
  print(value);
}

FutureOr<String> get value => true ? "1" : Future.value("1");

Dart Version

The issue was reproduced in version 2.17.3

@vgtle vgtle added the request Requests to resolve a particular developer problem label Aug 24, 2022
@eernstg
Copy link
Member

eernstg commented Aug 24, 2022

[Edit: The first version of my comment was wrong. Here's a better one.]

The static type of a conditional expression b ? e1 : e2 is computed as the standard upper bound of the static type of e1 and of e2, and the standard upper bound is computed by the function UP.

This case was actually discussed, and then omitted from UP, because we strive to avoid introducing FutureOr as the result of a computation. It's a source of confusion for the reader of the code if that type pops up without being mentioned in the code, and it creates a number of difficulties for type inference (so we'll have more cases where type inference can't find a solution).

The rules which we could have adopted go as follows:

  • UP(T1, Future<T2>) = FutureOr<T3> where T3 = UP(T1, T2).
  • UP(Future<T1>, T2) = FutureOr<T3> where T3 = UP(T1, T2).

We could also use a less permissive version:

  • UP(T, Future<T>) = FutureOr<T>.
  • UP(Future<T>, T) = FutureOr<T>.

These rules might be better at matching the cases where "it should obviously work", but it also takes a way a set of cases which are just as sound, and "almost as obvious". In the end we decided that it's better to omit these rules.

@eernstg eernstg transferred this issue from dart-lang/language Aug 24, 2022
@eernstg eernstg added request Requests to resolve a particular developer problem and removed request Requests to resolve a particular developer problem labels Aug 24, 2022
@eernstg eernstg transferred this issue from dart-lang/sdk Aug 24, 2022
@eernstg
Copy link
Member

eernstg commented Aug 24, 2022

@leafpetersen, we should have an old discussion about this in some issue, but I couldn't find it. Does it match your recollection of the situation?

@lrhn
Copy link
Member

lrhn commented Aug 24, 2022

This seems like our usual problem coming from not propagating the context type into the branches, and doing UP when there is a context type that both branches are (should be) assignable to.

If instead we pushed the context type into each branch, then we'll get a compile-time error if either branch is not assignable to the context type. If we don't, we know that both branches have types assignable to the context type, in which case no UP is needed, we can just use the context type as static type.

If there is no context type, we do need to find the UP of the two branches, but it can't be wrong then (just imprecise).

@eernstg
Copy link
Member

eernstg commented Aug 25, 2022

If instead we pushed the context type into each branch, then we'll get a
compile-time error if either branch is not assignable to the context type

The "try each branch separately" approach doesn't really help us inferring the type of x here:

var x = aBoolExpression ? "1" : Future.value("1");

This is of course an example of the case where there is no information in the context type, but the underlying problem is that we can't always try each branch separately when we just need to find a type for an expression. For instance, the context type could be non-trivial (here: X), and we still need to determine the overall type of the conditional expression:

List<X> listify<X>(X x) => [x];

void main() {
  var y = listify(aBoolExpression ? "1" : Future.value("1"));
}

The type of the expression listify(...) is the actual type argument inferred for the invocation, and that's just a type, not something which is amenable to a try-each-branch-separately strategy.

@lrhn
Copy link
Member

lrhn commented Aug 25, 2022

True, some context types do not provide a constraint, so they won't help with constraining the type of the branch expressions.
So, it's not so much the context type, as it's the context type constraint.
If there is no constraint, we can choose UP of the branch types as the static type.
If there is a constraint from the context, then pushing down the constraint will ensure that both branches satisfy that constraint before we do UP. Then we can choose the result of the UP as the static type, if it's a subtype of the context type constraint, otherwise use the constraint.

It will still find Object for var x = test ? Future<int>.value(0) : 0;, because we have no constraint and our UP won't give FutureOr. However, it will work for FutureOr<int> x = test ? Future<int>.value(0) : 0;, because we choose to use the context constraint of FutureOr<int> instead of Object found from UP(Future<int>, int). Because it's better.

@eernstg
Copy link
Member

eernstg commented Aug 25, 2022

It sounds like you're suggesting that we should add those cases to UP after all:

  • UP(T1, Future<T2>) = FutureOr<T3> where T3 = UP(T1, T2).
  • UP(Future<T1>, T2) = FutureOr<T3> where T3 = UP(T1, T2).

.. and then possibly say that they can only be used if the context type is FutureOr<P> for some type scheme P.

@ltOgt
Copy link

ltOgt commented Sep 1, 2022

we should have an old discussion about this in some issue, but I couldn't find it

Probably another issue, but maybe this: dart-lang/sdk#49553?

@eernstg
Copy link
Member

eernstg commented Sep 1, 2022

Probably another issue, but maybe this: dart-lang/sdk#49553?

Thanks for pointing that out! However, it's not quite the topic I was looking for: dart-lang/sdk#49553 is about the conditional operator ?:, but I was looking for an issue about the specific rules for UP where a FutureOr type is obtained from operand types T1 respectively Future<T2>, or Future<T1> respectively T2. But it doesn't matter much, because that old discussion was just one more location where the exact same rules were proposed.

@srawlins
Copy link
Member

I think this issue of letting the context type influence UP or LUB has come up recently... and I just ran into a case in internal code. It boils down to this:

Future<int> f(Future<int>? f, int x) async {
  return Future.value(f ?? x);
}

The argument type 'Object' can't be assigned to the parameter type 'FutureOr<int>?'.

@eernstg
Copy link
Member

eernstg commented Apr 25, 2024

Closing: The original example is now accepted by the analyzer and the common front end with no errors. The reason why the compile-time error has been eliminated is that we are now using this algorithm when computing the type of a conditional expression (?:).

The update is in the pipeline now, and it will be available in the beta and stable releases soon.

Note that we did not add a rule whereby UP will introduce of type of the form FutureOr<...> when none of the operands has that form. (Several comments above were arguing that we could and maybe even should do that. That was not a popular thing to do because we don't "invent" union types during type inference and similar steps.)

Instead, we're now using the context type as the resulting type if the upper bound returned by UP isn't assignable to the context type. This means that we can get the type FutureOr<...> from the context (so we haven't "invented" it), and then we just verify that this is a sound typing of the conditional expression.

This is even more powerful than improving on the behavior of UP in isolation because it allows the type of cond ? e1 : e2 to be a shared supertype of the type of e1 and the type of e2 even in cases where there is no reason to prefer that particular shared supertype. For example, there is no way UP could prefer I over J or vice versa in a situation like this:

class I {}
class J {}
class A implements I, J {}
class B implements I, J {}

var b = true;

void main() => b ? A() : B();

In this case the computed upper bound is Object because I isn't better than J, and vice versa. But if the context type is J then we can now choose J and make it succeed: J myJ = b ? A() : B(); // OK!.

@eernstg eernstg closed this as completed Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

5 participants