Add NativeDataResolver

Add NativeDataResolver

- to further align the NativeData computation towards separating builders from data.

R=sigmund@google.com

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

[Johnni Winther, 2017-03-04 16:34:28]

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

[Johnni Winther, 2017-03-04 16:34:29]

https://codereview.chromium.org/2732793002/diff/20001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/patch_resolver.dart (left):

https://codereview.chromium.org/2732793002/diff/20001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/patch_resolver.dart:36: } else if
(!compiler.backend.nativeData.isJsInterop(element)) {
We don't end up here if [element] is js interop.

https://codereview.chromium.org/2732793002/diff/20001/pkg/compiler/lib/src/pa...
File pkg/compiler/lib/src/patch_parser.dart (left):

https://codereview.chromium.org/2732793002/diff/20001/pkg/compiler/lib/src/pa...
pkg/compiler/lib/src/patch_parser.dart:336: checkNativeAnnotation(compiler,
patch);
We don't use @Native on patches.

[Siggi Cherem (dart-lang), 2017-03-14 04:09:15]

lgtm

https://codereview.chromium.org/2732793002/diff/20001/pkg/compiler/lib/src/pa...
File pkg/compiler/lib/src/patch_parser.dart (left):

https://codereview.chromium.org/2732793002/diff/20001/pkg/compiler/lib/src/pa...
pkg/compiler/lib/src/patch_parser.dart:336: checkNativeAnnotation(compiler,
patch);
On 2017/03/04 16:34:29, Johnni Winther wrote:
> We don't use @Native on patches.

should we add a check for it so we are not surprised if it doesn't work in the
future?

I'm OK punting this check though (It might not be worth it though - since once
we move to kernel we wont see patches anymore.)

https://codereview.chromium.org/2732793002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/2732793002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:667: if
(_nativeDataResolver.isJsInteropMember(element)) {
was the jsinterop check within patchResolver.resolveExternalFunction unreachable
because of this check here?

https://codereview.chromium.org/2732793002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/native_data.dart (right):

https://codereview.chromium.org/2732793002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/native_data.dart:36: bool
isJsInteropLibrary(LibraryElement element);
couple random thoughts here: 
- maybe this means we need to rename NativeClassData to something else?
NativeDeclarationData? NativeBasicData?
- isJsInteropLibrary could also mean is this the "dart:js" library or is this
the "package:js" library... The dartdoc clarifies this, so I don't think we need
to change it, but might be good to have this ambiguity in mind.

[Johnni Winther, 2017-03-15 10:54:54]

Description was changed from

==========
Add NativeDataResolver

- to further align the NativeData computation towards separating builders from
data.
==========

to

==========
Add NativeDataResolver

- to further align the NativeData computation towards separating builders from
data.

R=sigmund@google.com

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

[Johnni Winther, 2017-03-15 10:54:55]

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

[Johnni Winther, 2017-03-15 10:55:13]

https://codereview.chromium.org/2732793002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/backend.dart (right):

https://codereview.chromium.org/2732793002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/backend.dart:667: if
(_nativeDataResolver.isJsInteropMember(element)) {
On 2017/03/14 04:09:15, Siggi Cherem (dart-lang) wrote:
> was the jsinterop check within patchResolver.resolveExternalFunction
unreachable
> because of this check here?

Yes.

https://codereview.chromium.org/2732793002/diff/40001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/native_data.dart (right):

https://codereview.chromium.org/2732793002/diff/40001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/native_data.dart:36: bool
isJsInteropLibrary(LibraryElement element);
On 2017/03/14 04:09:15, Siggi Cherem (dart-lang) wrote:
> couple random thoughts here: 
> - maybe this means we need to rename NativeClassData to something else?
> NativeDeclarationData? NativeBasicData?
> - isJsInteropLibrary could also mean is this the "dart:js" library or is this
> the "package:js" library... The dartdoc clarifies this, so I don't think we
need
> to change it, but might be good to have this ambiguity in mind.

Acknowledged.