Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Downward inference into closures doesn't handle returns #308

Closed
vsmenon opened this issue Sep 1, 2015 · 7 comments
Closed

Downward inference into closures doesn't handle returns #308

vsmenon opened this issue Sep 1, 2015 · 7 comments

Comments

@vsmenon
Copy link
Contributor

vsmenon commented Sep 1, 2015

In the following, we get a strong more error on foo, but not on bar:

typedef int Int2Int(int x);
Int2Int foo() => (x) { return x + 1; };
Int2Int bar() => (x) => x + 1;

The following also doesn't trigger a warning:

int baz(x) { return x + 1; }

which feels inconsistent.

@leafpetersen Is this ultimately an ordering issue? It feels like we want to push down the param type before type checking the body.

@nex3 is hitting this when running strong mode on shelf.

@jmesserly
Copy link
Contributor

@vsmenon -- is this because we have better inference for arrow functions, compared with () { ... return ... }? I thought we used to do something different, although I wasn't able to find it with a quick grep through the code.

@vsmenon
Copy link
Contributor Author

vsmenon commented Sep 1, 2015

Yeah, here:

https://github.com/dart-lang/dev_compiler/blob/master/lib/src/checker/rules.dart#L790

We'd need to handle is! ExpressionFunctionBody rather than just return false.

@leafpetersen
Copy link
Contributor

Yes. I do downwards inference on functions, which is what allows your "bar" example to pass. If I'm inferring a function in a context expecting a T -> S, I check that the body is typable as an S. This may cause/require inference to happen within the body: for example, inferring the function (x) => [] in a context expecting a function returning a List<String> will cause the [] expression to be instantiated as a String list. The current checker does this inference in a kind of "duct-tape and bailing wire" fashion, which makes it a bit harder to do this for statement bodies. We could probably add code to handle some common statement body patterns, but doing it in full generality basically requires a full visitor (or else to integrate it more closely into one of the existing visitors, which is what I will probably do when integrating this into the analyzer).

@munificent
Copy link
Contributor

I'm working on getting an old project of mine error-free in strong mode and I hit this case pretty frequently. I would really like full return type inference for block-bodied lambdas and local function declarations. (I don't expect it for top-level fns because of recursion.)

I think this may be the most common source of strong mode warnings in that codebase.

@jmesserly
Copy link
Contributor

Yeah, thanks for the ping Bob. I personally haven't enabled strong_mode for DDC yet either(!). The three big blockers are this bug, lambda parameter inference (e.g. this affects the very common .map((x) => x.something).toList() pattern), and generic methods. Once we have those three, we'll be in a much better place.

@leafpetersen
Copy link
Contributor

@munificent
Copy link
Contributor

🎆

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants