fixes #27385, implement virtual fields in DDC

fixes #27385, implement virtual fields in DDC

If a field is marked @virtual in Analyzer, DDC will allow it to be overridden as well

also fixes #28114, abstract getters/setters broke generation of forwarding getters/setters

R=vsm@google.com

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

[Jennifer Messerly, 2016-12-14 21:44:27]

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

[vsm, 2016-12-14 22:30:47]

Nice!

One q, but otherwise lgtm!

https://codereview.chromium.org/2571363002/diff/20001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/class_property_model.dart (right):

https://codereview.chromium.org/2571363002/diff/20001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/class_property_model.dart:55: if
(accessor.isAbstract) continue;
I suspect this might break on the following:

abstract class A {
  int _x;

  int get x;
  void set x(int _val) { _x = _val; }
}

class B extends A {
  int get x => _x;
}

void main() {
  var a = new B();
  a.x = 42;
  print(a.x);
}

?

https://codereview.chromium.org/2571363002/diff/20001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/class_property_model.dart:79: if
(accessor.isAbstract) continue;
ditto?

[Jennifer Messerly, 2016-12-15 00:59:16]

https://codereview.chromium.org/2571363002/diff/20001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/class_property_model.dart (right):

https://codereview.chromium.org/2571363002/diff/20001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/class_property_model.dart:55: if
(accessor.isAbstract) continue;
On 2016/12/14 22:30:47, vsm wrote:
> I suspect this might break on the following:
> 
> abstract class A {
>   int _x;
> 
>   int get x;
>   void set x(int _val) { _x = _val; }
> }
> 
> class B extends A {
>   int get x => _x;
> }
> 
> void main() {
>   var a = new B();
>   a.x = 42;
>   print(a.x);
> }
> 
> ?

that example works.

We should uniformly ignore abstract getters/setters since they have no meaning
for generating concrete classes.

You do raise a good point: I think there may be a bug in how super accessors are
generated, however that bug would exist before the CL too. I'll see if I can get
a repro

https://codereview.chromium.org/2571363002/diff/20001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/class_property_model.dart:79: if
(accessor.isAbstract) continue;
On 2016/12/14 22:30:47, vsm wrote:
> ditto?

Acknowledged.

[Jennifer Messerly, 2016-12-15 01:07:29]

I filed this as https://github.com/dart-lang/sdk/issues/28114 but it's an easy
fix in code_generator so I'm going to include it in this CL

[Jennifer Messerly, 2016-12-15 01:08:04]

Description was changed from

==========
fixes #27385, implement virtual fields in DDC

If a field is marked @virtual in Analyzer, DDC will allow it to be overridden as
well
==========

to

==========
fixes #27385, implement virtual fields in DDC

If a field is marked @virtual in Analyzer, DDC will allow it to be overridden as
well

also fixes #28114, abstract getters/setters broke generation of forwarding
getters/setters
==========

[Jennifer Messerly, 2016-12-15 19:15:21]

PTAL, I've included a new test and a (very small) fix for 28114

[Jennifer Messerly, 2016-12-15 19:34:58]

Vijay just looked over my shoulder and LGTM'd the latest patchset :)

[Jennifer Messerly, 2016-12-15 19:39:33]

Description was changed from

==========
fixes #27385, implement virtual fields in DDC

If a field is marked @virtual in Analyzer, DDC will allow it to be overridden as
well

also fixes #28114, abstract getters/setters broke generation of forwarding
getters/setters
==========

to

==========
fixes #27385, implement virtual fields in DDC

If a field is marked @virtual in Analyzer, DDC will allow it to be overridden as
well

also fixes #28114, abstract getters/setters broke generation of forwarding
getters/setters

R=vsm@google.com

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

[Jennifer Messerly, 2016-12-15 19:39:34]

Committed patchset #4 (id:60001) manually as
77f53f609fd5dad2d5aa3bf6e69df0aadeea360e (presubmit successful).