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

Type of an if-null or conditional expression should only be the LUB if it's a subtype of the context #32339

Closed
stereotype441 opened this issue Feb 27, 2018 · 5 comments
Assignees
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. area-front-end Use area-front-end for front end / CFE / kernel format related issues. P4 type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Milestone

Comments

@stereotype441
Copy link
Member

Consider the following code:

class I {
  void f() {}
}
class J {}
class C extends I implements J {}
class D extends I implements J {}
void test(C c, D d) {
  I x = (c ?? d)..f();
}

The current analyzer and front end reject this code, because they consider the type of c ?? d to be LUB(C, D), which is Object, and Object does not define a method f.

According to #32291 (comment), the LUB should only be used if it is a subtype of the greatest closure of the context; otherwise the greatest closure of the context should be used. So the type of c ?? d should be I and the code should be accepted.

@stereotype441 stereotype441 added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-front-end Use area-front-end for front end / CFE / kernel format related issues. labels Feb 27, 2018
@stereotype441
Copy link
Member Author

We probably can't fix this until after #31792 is fixed, and here's why. Consider the following code:

class C {}
void test(C c, Object o) {
  C x = c ?? o;
}

Currently this gets compiled as C x = (c ?? o) as C;, which is not precisely what we want but has the correct runtime semantics. If we fix this bug without fixing #31792, then the type of c ?? o will be considered to be C, and the implicit downcast will be lost. We need to first fix #31792, so that the code will get compiled as C x = c ?? (o as C);.

@leafpetersen
Copy link
Member

Yes, this issue is essentially a clarification/refinement of #31792 as to what the inferred type of sub-expressions should be. The point of #31792 is that the inferred type of an expression should always be a subtype of its context. The point of this issue is that we should use the most precise type available that satisfies that (that is, if we are not forced to downcast, we should not upcast gratuitously).

@leafpetersen
Copy link
Member

leafpetersen commented Feb 28, 2018

Also, this is really about more than just the two operators mentioned, though that's the most visible place (and possibly the only place where the implementations aren't doing the right thing).

This is true in general whenever downwards inference "stops" and has to interact with upwards inference either by and downcast, or via a no-op because the upwards type is a subtype of the context type. Examples:

void foo(Object x) {}

// The static type of bar() should be resolved as int, not as Object, even though
// Object is the downwards type, since int <: Object.  This matters for completion
int bar() => 3 
foo(bar()..isEven);
void foo<T>(List<T> x) {}
List<int> l;
// This should infer <int> for the type parameter, even though the downwards
// context in which we infer l is List<?>.  That is, we should resolve l as having
// type List<int>, even though the greatest closure of the context is List<Object/dynamic>.
foo(l);

@kmillikin kmillikin added this to Incoming Untriaged in Dart Front End Apr 23, 2018
@leafpetersen leafpetersen self-assigned this Jun 22, 2018
@bwilkerson bwilkerson added the type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) label Sep 2, 2018
@bwilkerson bwilkerson added this to the Dart2.1 milestone Sep 2, 2018
@bwilkerson bwilkerson modified the milestones: Dart2.1, PostDart2.1 Sep 4, 2018
@aadilmaan aadilmaan modified the milestones: Future, D25 Release Jun 4, 2019
@srawlins srawlins added the analyzer-spec Issues with the analyzer's implementation of the language spec label Jun 17, 2020
@srawlins srawlins added the P4 label Jan 11, 2021
@srawlins
Copy link
Member

This is probably massively stale, but in case its not: @stereotype441 I swear I just saw a proposal or an issue from you regarding using context types in LUB. :)

@stereotype441
Copy link
Member Author

This is probably massively stale, but in case its not: @stereotype441 I swear I just saw a proposal or an issue from you regarding using context types in LUB. :)

Yeah, let's close this out in favor of dart-lang/language#1618.

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. area-front-end Use area-front-end for front end / CFE / kernel format related issues. P4 type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
Dart Front End
Incoming Untriaged
Development

No branches or pull requests

6 participants