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

improve strong mode inference error messages #29108

Closed
jmesserly opened this issue Mar 17, 2017 · 4 comments

Comments

@jmesserly
Copy link
Contributor

commented Mar 17, 2017

from @leafpetersen

The original error message is from this code:

values.fold(values.elementAt(0) as num, math.max)

The original error message is this:

[error] Couldn't infer type parameter 'T'.

Inferred type 'num' does not work with constraints:
  Argument 'combine' inferred as '(dynamic, dynamic) → dynamic' must be a '(T, dynamic) → T'.
The type 'num' was inferred from:
  Return type             declared as 'T'   used where a 'num' is required.
  Argument 'initialValue' inferred as 'num' must be a 'T'.

I ended up with this:

The type parameter 'T' was inferred to be 'num'
  Therefore argument 'combine' should be '(num, dynamic) → num', but
   the inferred type for 'combine' is '(dynamic, dynamic) → dynamic'.
The type 'num' was inferred for 'T' because:
  Return type             declared as 'T'   used where a 'num' is expected.

The changes are:

  • partially just wording: trying to avoid just saying things like "must be" or "is required', and instead try to give some intuition.
  • partially using the instantiated rather than uninstantiated type when talking about why the inference failed
  • In this case eliding the upwards constraints, which I think will be irrelevant with your new CL changes.
@jmesserly

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2017

I think we'd like to get this in 1.23 as it will improve usability of the error messages. (So, a really good tradeoff in terms of effort to user happiness.)

@bwilkerson

This comment has been minimized.

Copy link
Member

commented Mar 20, 2017

That would be great, but at this point the question might be: is it essential that this happen for 1.23 (that is, should we hold up the release because of this). From that perspective, I'd remove the milestone.

@jmesserly

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2017

yes. I will do it today, so hopefully this point becomes moot.

@jmesserly

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2017

@jmesserly jmesserly closed this in 4f9fff9 Mar 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.