Skip to content

Type resolution should use the downwards inference type #30542

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
leafpetersen opened this issue Aug 24, 2017 · 9 comments
Closed

Type resolution should use the downwards inference type #30542

leafpetersen opened this issue Aug 24, 2017 · 9 comments
Assignees
Labels
dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec language-strong-mode-polish legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Milestone

Comments

@leafpetersen
Copy link
Member

leafpetersen commented Aug 24, 2017

This code results in a no such method error on the call to bar on getBar():

class Foo {
  void bar() { print("hello"); }
}

class Bar {
}

dynamic getBar() => new Bar();
dynamic getFoo() => new Foo();

main() {
  Foo foo;
  foo = getFoo()..bar();
  /// The following gets treated like:
  /// foo = (getBar()..bar()) as Foo;
  /// instead of the more useful
  /// foo = (getBar() as Foo)..bar();
  foo = getBar()..bar();  
}

We seem to be correctly using a downwards inference context to fill in types in cascades, but resolution doesn't mark getBar() as having type Foo, with the associated implicit cast placed around the getFoo() call directly. As a result, we provide poor completion results.

In general, this applies to all composite structures like list literals, conditional expressions, etc. We will give better completion results and better user error checking if we push the type inwards.

@leafpetersen
Copy link
Member Author

cc @stereotype441 @jmesserly who have been discussing cast placement in kernel

@jmesserly
Copy link

yeah that seems like an oversight. Cascades should definitely be getting the context pushed down to the target. Especially since it's a common initialization pattern, which exactly when the context type is most useful .

@jmesserly
Copy link

jmesserly commented Aug 24, 2017

FYI @leafpetersen -- I also noticed bool context was not pushed down consistently, so I went ahead and fixed that. It's tied up in my CL that attempts to introduce T JS<T>(...) -- it became apparent that many things did not have a bool context as expected.

@leafpetersen
Copy link
Member Author

Yes, I never got a change to go through the language grammar and exhaustively make sure that we were using all of the contexts that were available, so I'm sure we're still missing a number of them. :(

@stereotype441
Copy link
Member

I think front_end already handles cascades correctly, but I'll double check.

@jmesserly, regarding bool, can you point me to your CL so I can verify that front_end implements the necessary bool contexts correctly as well?

@bwilkerson bwilkerson added P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Aug 25, 2017
@jmesserly jmesserly self-assigned this Aug 29, 2017
@jmesserly
Copy link

@stereotype441 -- yeah, I owe you that CL. It's caught up in one of my many outstanding DDC perf CL's, I'll try to disentangle that so I can send it out.

@jmesserly
Copy link

I think this is a duplicate of #31792

@jmesserly
Copy link

ah that one is for CFE; will reopen. I think you're looking at Analyzer though Leaf?

@aadilmaan aadilmaan added this to the Future milestone Jun 4, 2019
@srawlins srawlins added the dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec label Jun 17, 2020
@srawlins
Copy link
Member

No more errors on this code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dart-model-analyzer-spec Issues with the analyzer's implementation of the language spec language-strong-mode-polish legacy-area-analyzer Use area-devexp instead. P2 A bug or feature request we're likely to work on type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

7 participants