Skip to content

Better inference of conditional expressions. #1877

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

Closed
lrhn opened this issue Sep 30, 2021 · 4 comments
Closed

Better inference of conditional expressions. #1877

lrhn opened this issue Sep 30, 2021 · 4 comments
Labels
feature Proposed language feature that solves one or more problems least-upper-bound Proposals about improvements of the LUB algorithm

Comments

@lrhn
Copy link
Member

lrhn commented Sep 30, 2021

Currently:

class A<T> {}
class B extends A<int> {}
class C extends A<String> {}
A value = someTest ? B() : C();

fails to compile because the static type of someTest ? B() : C() is Object. It's Object because the LUB computation of the language spec is not particularly clever. That is what it is, but here it feels particularly disingenuous when we have an available context type which is also a valid upper bound (A<Object?> in this case).

So, could we say that the static type of e1 ? e2 : e3 with context type C is C if C is a supertype of the static types of e2 and e3, and only use LUB if there is no context type, or if the context type is not a supertype of the types of e2 and e3 (and then we'll likely have other issues too eventually).

See this StackOverflow question for context.

@lrhn lrhn added feature Proposed language feature that solves one or more problems inference labels Sep 30, 2021
@eernstg
Copy link
Member

eernstg commented Sep 30, 2021

Interesting!

However, in that case we probably want to ensure that a non-informative context type (with notation ? or _, depending on the age of the document) should prevent this rule from kicking in, it should not be considered as a top type.

There is also a circularity issue: If the type of the conditional expression will be used to select an implicitly induced declared type for a variable, or a function return type, etc., then it doesn't make sense to rely on that not-yet-inferred declared type during inference of the type of the conditional.

bool b = true;

void main() {
  var x = b ? 'true' : false; // Give `x` the type `Object` because that's the type of the conditional, because ...? ;-)
}

When this comes up I usually recommend that we compute the LUB as we do currently, but we start flagging that LUB (in conditional expressions, possibly in collection literals, maybe in a few other places) in the case where it produces Object or a top type, when that type will be used to infer a declared type of a variable or a function return type.

Perhaps it should only be when the LUB produces Object or a top type and none of the leaf operands has type Object or a top type. This will allow [1 as dynamic, 2, 3.5] to be a List<dynamic>; we did ask for that.

This would probably make it an analyzer hint.

@lrhn
Copy link
Member Author

lrhn commented Oct 19, 2022

Using the context type as static type should probably only be done when necessary. If we have a more precise upper bound, it may lose information to just use the context type.

num x = (test ? 0 : -1)..toRadixString(16).log(); // <- `log` is extension method on string.

If we promote the static type of test ? 0 : -1 to num eagerly, instead of using the LUB, then this code stops working.

So, proposal:

When inferring the type of `e` of the form `e1 ? e2 : e3`, with context type `C`, proceed as follows:
* Infer the type of `e1` to `T1` with context type `bool`. It's a compile-time error if the static type of `e1` is not assignable to `bool.`
* Infer the type of `e2` to `T2` with context type `C`.
* Infer the type of `e3` to `T2` with context type `C`.
* Let `S` be **UP**(`T1`, `T2`).
* If `C` is `_`, the static type of `e` is `S`.
* Otherwise, if `S` \< `C` (least /greatest closure of?), the static type of `e` is `S`
* Otherwise, if `T2` is assignable to (the ... closure of?) `C` and `T3` is assignable to (ditto) `C`, the static type of `e` is `C`.
* Otherwise the static type of `e` is `S`.

@eernstg
Copy link
Member

eernstg commented Apr 24, 2024

Closing: The original example here is accepted today by dart and dart analyze from c42fd69433ca0741fda5ecf82803aaa2fda24165. Also, the newly enabled feature (--enable-experiment=inference-update-3 until it was turned on by default) is doing exactly what is proposed in the initial posting:

So, could we say that the static type of e1 ? e2 : e3 with context type C is C if C is a supertype of the static types of e2 and e3, and only use LUB if there is no context type, or if the context type is not a supertype of the types of e2 and e3 (and then we'll likely have other issues too eventually).

@eernstg eernstg closed this as completed Apr 24, 2024
@lrhn
Copy link
Member Author

lrhn commented Apr 25, 2024

To be precise: We still use LUB even if there is a context type. We just discard the LUB result and use the context type, if the LUB result doesn't satisfy the context type requirement. The result can be better then the context type (for some definition of "better"), but not worse in a way that won't compile, which was the real surprise.
(Before this change, if the expression did compile, then its type stayed within the context type, and usually nobody noticed that it wasn't as precise as possible.)

The difference matters for something (very hypothetical) like num x = (test ? 0 : 255)..toRadixString(16).logString(); where an inferred type of num (the context type) for the conditional expression would not allow an int method to be called.
Probably not something that happens often, but it's possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems least-upper-bound Proposals about improvements of the LUB algorithm
Projects
None yet
Development

No branches or pull requests

2 participants