Implement AstBuilder integration for method/function invocations.

Implement AstBuilder integration for method/function invocations.

This is tricky because there isn't a clean correspondence between
analyzer's AST (which reflects user syntax) and kernel representation.
For example:

  f(args);

could be a call to a static function f, a method f in the current
class, or an invocation of a function-typed object stored in a
variable called f.  ResolutionStorer operates on the kernel
representation, so it sees the difference, but ResolutionApplier
operates on the analyzer AST, so it does not.  So we go to some extra
work to make sure a type is stored for f, regardless of whether f is a
function, method, or a variable.  This requires some re-ordering logic
in ResolutionStorer, since the inferred type of f is not known until
after the args have been visited.

The implementation is not complete; currently the type that we store
in the first two cases is `dynamic` because the type inference
mechanism doesn't preserve enough information to allow us to determine
the correct type; this will be remidied in a future CL.

A similar situation occurs for:

  x.m(args);

which could be a call to a method m in the object x, or an invocation
of a function-typed object returned by the getter m in the object x.

R=scheglov@google.com

Committed: https://github.com/dart-lang/sdk/commit/c057f091c211d119aec4bb3e89c06c5b4eb04a36

[Paul Berry, 2017-07-21 00:14:14]

paulberry@google.com changed reviewers:
+ scheglov@google.com

[scheglov, 2017-07-21 04:03:04]

LGTM

https://codereview.chromium.org/2984013002/diff/1/pkg/analyzer/lib/src/fasta/...
File pkg/analyzer/lib/src/fasta/resolution_applier.dart (right):

https://codereview.chromium.org/2984013002/diff/1/pkg/analyzer/lib/src/fasta/...
pkg/analyzer/lib/src/fasta/resolution_applier.dart:41:
node.methodName.staticType = _getTypeFor(node.methodName);
Could we move this statement after arguments, so avoid deferring types in
storer?

[Paul Berry, 2017-07-21 17:40:11]

https://codereview.chromium.org/2984013002/diff/1/pkg/analyzer/lib/src/fasta/...
File pkg/analyzer/lib/src/fasta/resolution_applier.dart (right):

https://codereview.chromium.org/2984013002/diff/1/pkg/analyzer/lib/src/fasta/...
pkg/analyzer/lib/src/fasta/resolution_applier.dart:41:
node.methodName.staticType = _getTypeFor(node.methodName);
On 2017/07/21 04:03:03, scheglov wrote:
> Could we move this statement after arguments, so avoid deferring types in
> storer?

Unfortunately, your suggestion would break a different case:

void test(f) {
  f(args);
}

In this case, the analyzer AST treats `f(args)` as a method invocation with no
target and a methodName of `f`, but kernel treats it as a method invocation with
a target of `f` (kernel calles it the "receiver") and a method name of `call`,
i.e. equivalent to `f.call(args);`.  So we would still have to reorder types in
storer, we would just have to reorder these implicit "call" cases instead of
reordering ordinary method calls.

I spent some time working out both cases on pen and paper, and IMHO reordering
ordinary method calls is less work.

[scheglov, 2017-07-21 17:43:58]

Still LGTM.

https://codereview.chromium.org/2984013002/diff/1/pkg/analyzer/lib/src/fasta/...
File pkg/analyzer/lib/src/fasta/resolution_applier.dart (right):

https://codereview.chromium.org/2984013002/diff/1/pkg/analyzer/lib/src/fasta/...
pkg/analyzer/lib/src/fasta/resolution_applier.dart:41:
node.methodName.staticType = _getTypeFor(node.methodName);
On 2017/07/21 17:40:11, Paul Berry wrote:
> On 2017/07/21 04:03:03, scheglov wrote:
> > Could we move this statement after arguments, so avoid deferring types in
> > storer?
> 
> Unfortunately, your suggestion would break a different case:
> 
> void test(f) {
>   f(args);
> }
> 
> In this case, the analyzer AST treats `f(args)` as a method invocation with no
> target and a methodName of `f`, but kernel treats it as a method invocation
with
> a target of `f` (kernel calles it the "receiver") and a method name of `call`,
> i.e. equivalent to `f.call(args);`.  So we would still have to reorder types
in
> storer, we would just have to reorder these implicit "call" cases instead of
> reordering ordinary method calls.
> 
> I spent some time working out both cases on pen and paper, and IMHO reordering
> ordinary method calls is less work.

OK, thank you for the explanation.

[Paul Berry, 2017-07-21 17:47:30]

Description was changed from

==========
Implement AstBuilder integration for method/function invocations.

This is tricky because there isn't a clean correspondence between
analyzer's AST (which reflects user syntax) and kernel representation.
For example:

  f(args);

could be a call to a static function f, a method f in the current
class, or an invocation of a function-typed object stored in a
variable called f.  ResolutionStorer operates on the kernel
representation, so it sees the difference, but ResolutionApplier
operates on the analyzer AST, so it does not.  So we go to some extra
work to make sure a type is stored for f, regardless of whether f is a
function, method, or a variable.  This requires some re-ordering logic
in ResolutionStorer, since the inferred type of f is not known until
after the args have been visited.

The implementation is not complete; currently the type that we store
in the first two cases is `dynamic` because the type inference
mechanism doesn't preserve enough information to allow us to determine
the correct type; this will be remidied in a future CL.

A similar situation occurs for:

  x.m(args);

which could be a call to a method m in the object x, or an invocation
of a function-typed object returned by the getter m in the object x.
==========

to

==========
Implement AstBuilder integration for method/function invocations.

This is tricky because there isn't a clean correspondence between
analyzer's AST (which reflects user syntax) and kernel representation.
For example:

  f(args);

could be a call to a static function f, a method f in the current
class, or an invocation of a function-typed object stored in a
variable called f.  ResolutionStorer operates on the kernel
representation, so it sees the difference, but ResolutionApplier
operates on the analyzer AST, so it does not.  So we go to some extra
work to make sure a type is stored for f, regardless of whether f is a
function, method, or a variable.  This requires some re-ordering logic
in ResolutionStorer, since the inferred type of f is not known until
after the args have been visited.

The implementation is not complete; currently the type that we store
in the first two cases is `dynamic` because the type inference
mechanism doesn't preserve enough information to allow us to determine
the correct type; this will be remidied in a future CL.

A similar situation occurs for:

  x.m(args);

which could be a call to a method m in the object x, or an invocation
of a function-typed object returned by the getter m in the object x.

R=scheglov@google.com

Committed:
https://github.com/dart-lang/sdk/commit/c057f091c211d119aec4bb3e89c06c5b4eb04a36
==========

[Paul Berry, 2017-07-21 17:47:31]

Committed patchset #1 (id:1) manually as
c057f091c211d119aec4bb3e89c06c5b4eb04a36 (presubmit successful).