Change MethodInvocation.interfaceTarget from a Procedure to a Member.

Change MethodInvocation.interfaceTarget from a Procedure to a Member.

It's possible for the interface target to be a field, e.g.:

    typedef void F();
    class C {
      F f;
    }
    void g(C c) {
      c.f(); // Interface target is C::f
    }

Also, this CL fixes MethodInvocation.getStaticType() in the case where
the interface target is a getter.

R=dmitryas@google.com, kmillikin@google.com, scheglov@google.com

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

[Paul Berry, 2017-06-05 21:25:38]

paulberry@google.com changed reviewers:
+ dmitryas@google.com, kmillikin@google.com, scheglov@google.com

[Paul Berry, 2017-06-05 21:25:38]

Konstantin, can you do a preliminary review since you're in US pacific time?  I
won't land until I have approval from Dmitry or Kevin.

[scheglov, 2017-06-05 21:30:48]

LGTM

[Paul Berry, 2017-06-06 03:01:40]

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

[Paul Berry, 2017-06-06 03:01:40]

+jensj since Kevin is on vacation

[jensj, 2017-06-06 06:51:38]

Sorry, but I don't feel qualified to review this.
(For one thing I have no context of what we're talking about here.).
Kevin should return tomorrow - hopefully that'll do.

[Dmitry Stefantsov, 2017-06-06 07:39:18]

LGTM. However, I'd wait for Kevin's opinion on that, because the changes may
interfere with type inference, and he's working on something related AFAIK.

[ahe, 2017-06-06 11:21:18]

ahe@google.com changed reviewers:
+ ahe@google.com

[ahe, 2017-06-06 11:21:18]

Would it make more sense to change how Fasta generates code for this example?

If we know the interface target, we can change this to a PropertyGet followed by
a .call.

If it isn't feasible to make this information available to Fasta in general,
then I think this solution is the only other option.

[Paul Berry, 2017-06-06 16:00:50]

On 2017/06/06 11:21:18, ahe wrote:
> Would it make more sense to change how Fasta generates code for this example?
> 
> If we know the interface target, we can change this to a PropertyGet followed
by
> a .call.

That's a good question.  I guess it depends on whether back-ends will want to
generate different code for the two cases.

Note that in the dynamic dispatch case the generated code will have to handle
both possibilities, because the code might do this:

    typedef void F();
    class A {
      F f;
    }
    class B {
      void f() { ... }
    }
    void foo(dynamic x) {
      x.f();
    }
    main() {
      foo(new A()); // foo does a property get of A.f followed by a .call
      foo(new B()); // foo does a call to B.f()
    }

Since the VM (currently) treats everything as dynamic and optimizes later based
on information gathered at runtime, it probably wants to treat the code the same
in both cases.  I'm not so sure about DDC and Dart2js.

I'm happy to wait for Kevin's opinion on this.

[Paul Berry, 2017-06-07 14:01:37]

On 2017/06/06 16:00:50, Paul Berry wrote:
> On 2017/06/06 11:21:18, ahe wrote:
> > Would it make more sense to change how Fasta generates code for this
example?
> > 
> > If we know the interface target, we can change this to a PropertyGet
followed
> by
> > a .call.
> 
> That's a good question.  I guess it depends on whether back-ends will want to
> generate different code for the two cases.
> 
> Note that in the dynamic dispatch case the generated code will have to handle
> both possibilities, because the code might do this:
> 
>     typedef void F();
>     class A {
>       F f;
>     }
>     class B {
>       void f() { ... }
>     }
>     void foo(dynamic x) {
>       x.f();
>     }
>     main() {
>       foo(new A()); // foo does a property get of A.f followed by a .call
>       foo(new B()); // foo does a call to B.f()
>     }
> 
> Since the VM (currently) treats everything as dynamic and optimizes later
based
> on information gathered at runtime, it probably wants to treat the code the
same
> in both cases.  I'm not so sure about DDC and Dart2js.
> 
> I'm happy to wait for Kevin's opinion on this.

Another thought: there's a (weak) argument in favor of this change based on
consistency: PropertyGet.interfaceTarget and PropertySet.interfaceTarget accept
any sort of member.  It might be nice to make MethodInvocation.interfaceTarget
consistent with that.

[Kevin Millikin (Google), 2017-06-07 15:11:14]

I do like Peter's suggestion, but consistency is a strong argument.  Maybe we'll
decide to change it later but this looks right for now.

LGTM.

[Paul Berry, 2017-06-07 16:20:56]

Description was changed from

==========
Change MethodInvocation.interfaceTarget from a Procedure to a Member.

It's possible for the interface target to be a field, e.g.:

    typedef void F();
    class C {
      F f;
    }
    void g(C c) {
      c.f(); // Interface target is C::f
    }

Also, this CL fixes MethodInvocation.getStaticType() in the case where
the interface target is a getter.
==========

to

==========
Change MethodInvocation.interfaceTarget from a Procedure to a Member.

It's possible for the interface target to be a field, e.g.:

    typedef void F();
    class C {
      F f;
    }
    void g(C c) {
      c.f(); // Interface target is C::f
    }

Also, this CL fixes MethodInvocation.getStaticType() in the case where
the interface target is a getter.

R=dmitryas@google.com, kmillikin@google.com, scheglov@google.com

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

[Paul Berry, 2017-06-07 16:20:58]

Committed patchset #2 (id:20001) manually as
5c3dd4b11b2c9665b263ad8a0bde9e96f7f2529b (presubmit successful).