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

implement generic functions/methods in other AST nodes besides MethodInvocation #25175

Closed
3 of 5 tasks
jmesserly opened this issue Dec 8, 2015 · 13 comments
Closed
3 of 5 tasks
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. closed-duplicate Closed in favor of an existing report P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug

Comments

@jmesserly
Copy link

Right now generic function calls are only supported for the MethodInvocation AST node, which applies to method calls like o.m<t>(...) and function calls like f<t>(...)

We need to support these:

  • tear offs, e.g. o.m
  • instantiated tear offs, e.g. o.m<t>
  • instantiation of the function type, e.g. var y = f<int>;
  • function expression invocation, e.g. (f)(1, 2) or (f)<int>(1, 2)
  • generic function expression, e.g. <T>(T x) => x
@jmesserly jmesserly added P1 A high priority bug; for example, a single project is unusable or has many test failures area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-strong-mode labels Dec 8, 2015
@jmesserly jmesserly changed the title generic methods don't work yet for FunctionExpressionInvocation generic methods don't work yet for tear offs, FunctionExpressionInvocation Dec 8, 2015
@jmesserly jmesserly changed the title generic methods don't work yet for tear offs, FunctionExpressionInvocation implement generic functions/methods in other AST nodes besides MethodInvocation Dec 8, 2015
@leafpetersen leafpetersen self-assigned this Dec 8, 2015
@jmesserly
Copy link
Author

btw feel free to edit the first message, not sure if I've captured all of the missing features

@leafpetersen
Copy link
Member

Tear offs seem to be working. I think the partial instantiation is lower priority, particularly given that I think the work involved will be larger. I'll work on getting function expression invocation working.

@jmesserly
Copy link
Author

@leafpetersen if you don't mind I'll take this, I think it will be pretty easy to fix to FunctionExpressionInvocation after the cleanup in https://codereview.chromium.org/1568643002/

@jmesserly jmesserly assigned jmesserly and unassigned leafpetersen Jan 6, 2016
@jmesserly
Copy link
Author

This might belong in a different bug, but I just noticed we don't handle this case correctly:

int lambdaCall = (/*<T>*/(/*=T*/ e) => e)/*<int>*/(3);

@leafpetersen
Copy link
Member

Sure, all yours.

@jmesserly
Copy link
Author

@jmesserly
Copy link
Author

Hmmm, actually it looks like we don't support that lambda form?
https://github.com/leafpetersen/dep-generic-methods/blob/master/proposal.md#functions

In which case it's a bug in the error handling, as we should disallow type parameters there.

jmesserly pushed a commit that referenced this issue Jan 7, 2016
Introduces MethodInvocation.staticInvokeType to track the type of this invocation. This provides a natural place to store the instantiated generic function type. We then use this type when we are computing corresponding parameters or return types. As a result we do not need FunctionMember, and some of the code around generics becomes simpler/more uniform.

This approach should be a straightforward way to add support for generics in FunctionExpressionInvocation, in a follow up (see issue #25175).

Also renames "boundTypeParameters" to "typeFormals". "formals" and "actuals" is a pretty common way to describe these, I'm not sure why I didn't think of that better name originally. :)

Also removes broken ParameterMember.== that I had added in a previous CL. And the broken FunctionTypeImpl.originalFunction/instantiatedTypeArguments getters.

Finally, this fixes #23252 in the process of adding this. The return type of a "call" method was not being statically analyzed if the target was a VariableElement.

R=brianwilkerson@google.com, leafp@google.com

Review URL: https://codereview.chromium.org/1568643002 .
@leafpetersen
Copy link
Member

Tearoffs were fixed in https://codereview.chromium.org/1568643002

jmesserly pushed a commit that referenced this issue Jan 11, 2016
fixes part of #25175

R=brianwilkerson@google.com, leafp@google.com

Review URL: https://codereview.chromium.org/1561233003 .
@jmesserly
Copy link
Author

Fix for lambdas: https://codereview.chromium.org/1579303002/

Funny to chase down the bug, only to see a TODO(jmesserly) describing the problem :)

jmesserly pushed a commit that referenced this issue Jan 13, 2016
@jmesserly jmesserly added Priority-Medium and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jan 13, 2016
@jmesserly
Copy link
Author

dropping priority, as the only cases left are the instantiated tear-offs & functions.

@jmesserly
Copy link
Author

There's an interesting instantiated tear off example at dart-archive/dev_compiler#426. It uses a generic function as a default argument. For that example, we'd also have to allow function instantiations to be treated as constant expressions.

@jmesserly
Copy link
Author

CL is out for downward inference on instantiations. So even if we don't have syntax support yet, at least they will work in an inferred context. See #25619

@munificent munificent mentioned this issue Feb 18, 2016
35 tasks
@jmesserly
Copy link
Author

split out the remaining issues into a new bug: #25824, as they're lower priority than what this bug originally covered

@jmesserly jmesserly added the closed-duplicate Closed in favor of an existing report label Feb 19, 2016
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug and removed Priority-Medium labels Mar 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. closed-duplicate Closed in favor of an existing report P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants