fix #26896, mark all @proxy ops as dynamic

fix #26896, mark all @proxy ops as dynamic

This was already working for calls/methods because we fail to get a FunctionType. But it wasn't working for other cases like operators.

R=vsm@google.com

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

[Jennifer Messerly, 2016-08-09 00:06:47]

jmesserly@google.com changed reviewers:
+ vsm@google.com

[vsm, 2016-08-09 15:20:51]

https://codereview.chromium.org/2223343002/diff/20001/pkg/analyzer/lib/src/ta...
File pkg/analyzer/lib/src/task/strong/checker.dart (right):

https://codereview.chromium.org/2223343002/diff/20001/pkg/analyzer/lib/src/ta...
pkg/analyzer/lib/src/task/strong/checker.dart:1035: (element is ClassElement &&
element.isOrInheritsProxy);
Is this too conservative if there is an actual method on the class?

E.g., foo vs bar below:

@Proxy()
class A {
  void foo(int x) { ... }
  noSuchMethod(...) { ... }
}

var a = new A();
a.foo(42); // Not dynamic
a.bar(42); // Dynamic

[Jennifer Messerly, 2016-08-09 15:31:11]

https://codereview.chromium.org/2223343002/diff/20001/pkg/analyzer/lib/src/ta...
File pkg/analyzer/lib/src/task/strong/checker.dart (right):

https://codereview.chromium.org/2223343002/diff/20001/pkg/analyzer/lib/src/ta...
pkg/analyzer/lib/src/task/strong/checker.dart:1035: (element is ClassElement &&
element.isOrInheritsProxy);
On 2016/08/09 15:20:51, vsm wrote:
> Is this too conservative if there is an actual method on the class?
> 
> E.g., foo vs bar below:
> 
> @Proxy()
> class A {
>   void foo(int x) { ... }
>   noSuchMethod(...) { ... }
> }
> 
> var a = new A();
> a.foo(42); // Not dynamic
> a.bar(42); // Dynamic

This path actually only handles operators. Method/function calls were already
handled by _isDynamicCall, and they use the absence of an Element to figure out
that it must be a dynamic call.

For operators we might be able to use a similar technique. It would mean
reworking _isDynamicTarget, removing it and treating operators as dynamic calls.
Not sure how well that would work but it is the only approach I think that would
address your concern here. But you're right, we're a tad conservative on using
operators on @proxy classes.

[Jennifer Messerly, 2016-08-09 15:36:21]

On 2016/08/09 15:31:11, John Messerly wrote:
>
https://codereview.chromium.org/2223343002/diff/20001/pkg/analyzer/lib/src/ta...
> File pkg/analyzer/lib/src/task/strong/checker.dart (right):
> 
>
https://codereview.chromium.org/2223343002/diff/20001/pkg/analyzer/lib/src/ta...
> pkg/analyzer/lib/src/task/strong/checker.dart:1035: (element is ClassElement
&&
> element.isOrInheritsProxy);
> On 2016/08/09 15:20:51, vsm wrote:
> > Is this too conservative if there is an actual method on the class?
> > 
> > E.g., foo vs bar below:
> > 
> > @Proxy()
> > class A {
> >   void foo(int x) { ... }
> >   noSuchMethod(...) { ... }
> > }
> > 
> > var a = new A();
> > a.foo(42); // Not dynamic
> > a.bar(42); // Dynamic
> 
> This path actually only handles operators. Method/function calls were already
> handled by _isDynamicCall, and they use the absence of an Element to figure
out
> that it must be a dynamic call.
> 
> For operators we might be able to use a similar technique. It would mean
> reworking _isDynamicTarget, removing it and treating operators as dynamic
calls.
> Not sure how well that would work but it is the only approach I think that
would
> address your concern here. But you're right, we're a tad conservative on using
> operators on @proxy classes.

anyway I'll add a test and try and fix it.

[Jennifer Messerly, 2016-08-09 15:45:56]

PTAL. refactors to use elements.

(Amusing blast from the past:
https://github.com/dart-lang/dev_compiler/issues/132)

[vsm, 2016-08-09 15:50:30]

lgtm!

[Jennifer Messerly, 2016-08-09 17:15:11]

Description was changed from

==========
fix #26896, mark all @proxy ops as dynamic

This was already working for calls/methods because we fail to get a
FunctionType. But it wasn't working for other cases like operators.
==========

to

==========
fix #26896, mark all @proxy ops as dynamic

This was already working for calls/methods because we fail to get a
FunctionType. But it wasn't working for other cases like operators.

R=vsm@google.com

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

[Jennifer Messerly, 2016-08-09 17:15:12]

Committed patchset #3 (id:40001) manually as
eafe459b0e72ecd4803d6c7616b3d67477a22b8a (presubmit successful).