Skip to content

Type inference of function return value #33137

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
pd4d10 opened this issue May 16, 2018 · 7 comments
Closed

Type inference of function return value #33137

pd4d10 opened this issue May 16, 2018 · 7 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).

Comments

@pd4d10
Copy link

pd4d10 commented May 16, 2018

  • Dart SDK Version: 2.0.0-dev.43.0
  • macOS

Considering a simple function like this:

add(int a, int b) {
  return a + b;
}

VSCode(with Dart extension) tells me the return type of add function is dynamic:

add(int a, int b) → dynamic

But if we add a wrong type, like

String add(int a, int b) {
  return a + b;
}

Then it shows an error:

[dart] The return type 'int' isn't a 'String', as defined by the method 'add'.

Obviously the compiler knows the return value is a int, but if we don't declare it explicitly, it will be a dynamic. Is it by design?

@vsmenon vsmenon added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label May 16, 2018
@leafpetersen
Copy link
Member

This is by design. We do infer return types of non-recursive local functions (functions declared inside of the scope of another function or method), but for top level functions and methods, we do not infer return types (except via override inference). The reasons are as follows:

  • Methods and top level functions are usually part of the API of a program, and it's valuable to be able to quickly read off the API of a piece of code. Doing method body based return type inference means that understanding the signature of the API requires reading through the method body.
  • Methods and top level functions can be arbitrarily mutually recursive, which makes the inference problem much harder and more expensive.

For primarily these reasons, we do not infer return types for top level functions and methods. Leaving off the return type is just another way of saying dynamic.

@maks
Copy link

maks commented Jul 31, 2019

@leafpetersen apologies to dig up such an old issue as wasn't sure if this warranted a new issue, but would those reasons for not inferring method types still apply to something like:

AuthBloc _authBloc(BuildContext context) => Provider.of<AuthBloc>(context);

this recently cuaght me out as I expected the analyzer to infer the return type of _authBloc() but it did not.
Also as a side note it did not infer the type of context in _authBloc(context) either.

@leafpetersen
Copy link
Member

but would those reasons for not inferring method types still apply to something like:

Well, we could certainly do return type inference for non-recursive functions, if that's what you mean. It's a bit finicky to say what we mean by "recursive" though. You make the definition syntactic (compute the connected components by free variables), or more semantic ("do I need to reference the type of f in order to compute the type of g and vice versa"), or whatever. All of those have their issues, but could be worked out.

I think to make that have non-surprising behavior, we'd probably want to make it an error if inference couldn't be done. For example, suppose we chose the first definition of recursive. Then this code:

  g(int x) {
    return 3;
  }
  f(int x) {
    g(x);
    return g(x);
 }

happily infers int for both return types. But now if I change the definition of g to be:

  g(int x) {
    if (x > 0) f(x -1);
    return 3;
  }

then suddenly the two types form a connected component, and are hence considered mutually recursive for inference. If we didn't make that an error, then adding such a call to f would suddenly change the return types to dynamic, which would probably "work" but would be very surprising.

So if we made such a change, we would probably want to make it an error if any functions in a connected component did not declare explicit return types. This seems reasonable to me on brief thought, but is a significant breaking change.

The usage profile of Dart users seems to be continuing to skew towards a preference for static typing + inference (over its more dynamic origins), so at some point I do think it would be good for us to consider making inference a bit stronger. It would need to be managed as an opt-in breaking language version release though.

Also as a side note it did not infer the type of context in _authBloc(context) either.

Not sure what you mean by this - do you mean that the type of the parameter to _authBloc was not inferred? In general we currently never do any inference of variables based on their uses.

@maks
Copy link

maks commented Jul 31, 2019

@leafpetersen Thanks for the really excellent and detailed explaination!
It all makes good sense. I think I was really only considering the special case of the arrow notation, where the return type of expression is known, but I don't know how this looks to the analyzer so perhaps thats not really practical.
And yes what I meant about the type of context was inference by its usage, sorry I hadn't known that Dart didnt do that, thinking about it, I can see thats a hard problem to in general case.

Also I'd agree with your view of the Dart users, I'm definitely one of those new comers to Dart thru Flutter, having a Java and more recently Kotlin background and anecdotally in our local Flutter usergroup that seems to be very common and several of us have discussed being surprised by this sort of thing when coming from "stricter" world of Kotlin.

I actually came across this issue because I only discovered now, after several months of Flutter usage the availability of https://dart.dev/guides/language/analysis-options#enabling-additional-type-checks and specifically implicit-dynamic: false. In some ways I think this is a tooling issue, as using VS Code without it, I couldn't immediately see the difference between type inference working successfully vs it using implicit synamic, so turning on implicit-dynamic: false has been extremely helpful. So perhaps having another analyzer flag to enable the stronger type inference as an opt-in would be possible in the future?

@leafpetersen
Copy link
Member

I think I was really only considering the special case of the arrow notation, where the return type of expression is known

Here's the same example as above using arrow functions:

  T seq<T>(Object x, T y) => y;
  g(int x) => 3;   
  f(int x) => g(x);

Where changing the definition of g to seq((x > 0) ? f(x-1) : null, 3) will cause the same change.

I'm definitely one of those new comers to Dart thru Flutter, having a Java and more recently Kotlin background and anecdotally in our local Flutter usergroup that seems to be very common and several of us have discussed being surprised by this sort of thing when coming from "stricter" world of Kotlin.

Good to know, thanks.

So perhaps having another analyzer flag to enable the stronger type inference as an opt-in would be possible in the future?

@srawlins is doing some work to provide stronger opt in checking. The upcoming NNBD release will also take some big steps that way. Beyond that, it's hard to say, but we certainly appreciate, and listen to, the feedback.

@srawlins
Copy link
Member

srawlins commented Aug 1, 2019

So perhaps having another analyzer flag to enable the stronger type inference as an opt-in would be possible in the future?

I think that strict-inference will help in the case of the function return types and parameters, but its not implemented. It's still an experimental flag, and I'll be landing that support in the next month or two, so you could add this to your analysis-options, if you'd like to stay tuned :)

analyzer:
  language:
      strict-inference: true

@maks
Copy link

maks commented Aug 2, 2019

@srawlins thank you so much! I'll definitely use this and look out fo rthe implementation in the future, I've subscribed to #33749 which I guess is tracking that.

Also again thanks for responding to me feedback so quickly and yes I am very much looking forward to the release that bring NNBD!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).
Projects
None yet
Development

No branches or pull requests

5 participants