Add equivalence check of NativeData and InterceptorData

Add equivalence check of NativeData and InterceptorData

Extracted NativeMemberResolver from NativeDataResolver and added
KernelNativeMemberResolver to resolve native member data before
world impact computation.

R=sigmund@google.com

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

[Johnni Winther, 2017-05-10 11:49:46]

johnniwinther@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2017-05-10 16:50:05]

lgtm

https://codereview.chromium.org/2873113004/diff/1/tests/compiler/dart2js/seri...
File tests/compiler/dart2js/serialization/model_test_helper.dart (right):

https://codereview.chromium.org/2873113004/diff/1/tests/compiler/dart2js/seri...
tests/compiler/dart2js/serialization/model_test_helper.dart:215:
data2.nativeMethodBehavior,
not for this CL, but I keep trying to think of ways in which we can make all of
these "check" functions easier to maintain. Probably not worthwhile?

Here are some ideas:
 1. avoid the duplication with data1 and data2, for example, maybe we can write
the field as a closure:

   checkMapEquivalence(
       data1,
       data2,
      'nativeMethodBehavior',
      (d) => d.nativeMethodBehaviod,
       ...)

 2. create some kind of descriptor for the data that you are checking, and
provide all equivalence functions once with that descriptor? For example:

  var descriptor = new Descritor('native-data')
     ..mapCheck(
         'nativeMethodBehavior',
         (d) => d.nativeMethodBehavior.
         keysType: Entity,
         valuesType: NativeBehavior),
     ..mapCheck(
         'nativeFieldLoadBehavior',
         (d) => d.nativeFieldLoadBehavior));


  runChecks(descriptor, data1, data2, {
     Entity: strategy.elementEquivalence,
     NativeBehavior: testNativeBehavior
  });

 3. even further - we could use mirrors or a code generator to make this more
succinct. With mirrors we would declare this descriptor using metadata, for
example:

   class NativeDataEquivalenceChecks extends EquivalenceChecks {
      NativeData data;
      NativeDataEquivalenceChecks(this.data);

      @Check.map(Entity, NativeBehavior)
      get nativeMethodBehavior => data.nativeMethodBehavior;

      @Check.set(Entity)
      get anonymousJsInteropClasses => data.jsInteropClasses;

      ..
   }

[Siggi Cherem (dart-lang), 2017-05-10 16:50:30]

also - should we move model/test_helper out of serialization? seems like
equivalence is independent of the serialization tests

[Johnni Winther, 2017-05-11 12:01:27]

On 2017/05/10 16:50:30, Siggi Cherem (dart-lang) wrote:
> also - should we move model/test_helper out of serialization? seems like
> equivalence is independent of the serialization tests

Yes, a general reorganization of the helpers files and content is needed!

[Johnni Winther, 2017-05-11 12:01:33]

https://codereview.chromium.org/2873113004/diff/1/tests/compiler/dart2js/seri...
File tests/compiler/dart2js/serialization/model_test_helper.dart (right):

https://codereview.chromium.org/2873113004/diff/1/tests/compiler/dart2js/seri...
tests/compiler/dart2js/serialization/model_test_helper.dart:215:
data2.nativeMethodBehavior,
On 2017/05/10 16:50:05, Siggi Cherem (dart-lang) wrote:
> not for this CL, but I keep trying to think of ways in which we can make all
of
> these "check" functions easier to maintain. Probably not worthwhile?
> 
> Here are some ideas:
>  1. avoid the duplication with data1 and data2, for example, maybe we can
write
> the field as a closure:
> 
>    checkMapEquivalence(
>        data1,
>        data2,
>       'nativeMethodBehavior',
>       (d) => d.nativeMethodBehaviod,
>        ...)
> 
>  2. create some kind of descriptor for the data that you are checking, and
> provide all equivalence functions once with that descriptor? For example:
> 
>   var descriptor = new Descritor('native-data')
>      ..mapCheck(
>          'nativeMethodBehavior',
>          (d) => d.nativeMethodBehavior.
>          keysType: Entity,
>          valuesType: NativeBehavior),
>      ..mapCheck(
>          'nativeFieldLoadBehavior',
>          (d) => d.nativeFieldLoadBehavior));
> 
> 
>   runChecks(descriptor, data1, data2, {
>      Entity: strategy.elementEquivalence,
>      NativeBehavior: testNativeBehavior
>   });
> 
>  3. even further - we could use mirrors or a code generator to make this more
> succinct. With mirrors we would declare this descriptor using metadata, for
> example:
> 
>    class NativeDataEquivalenceChecks extends EquivalenceChecks {
>       NativeData data;
>       NativeDataEquivalenceChecks(this.data);
> 
>       @Check.map(Entity, NativeBehavior)
>       get nativeMethodBehavior => data.nativeMethodBehavior;
> 
>       @Check.set(Entity)
>       get anonymousJsInteropClasses => data.jsInteropClasses;
> 
>       ..
>    }

Acknowledged.

[Johnni Winther, 2017-05-11 12:11:17]

Description was changed from

==========
Add equivalence check of NativeData and InterceptorData

Extracted NativeMemberResolver from NativeDataResolver and added
KernelNativeMemberResolver to resolve native member data before
world impact computation.
==========

to

==========
Add equivalence check of NativeData and InterceptorData

Extracted NativeMemberResolver from NativeDataResolver and added
KernelNativeMemberResolver to resolve native member data before
world impact computation.

R=sigmund@google.com

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

[Johnni Winther, 2017-05-11 12:11:18]

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