Handle instantiation of native classes in closed_world2_test

Handle instantiation of native classes in closed_world2_test

R=sigmund@google.com

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

[Johnni Winther, 2017-04-25 10:35:24]

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

[Siggi Cherem (dart-lang), 2017-04-25 19:45:54]

lgtm

https://codereview.chromium.org/2836993003/diff/1/pkg/compiler/lib/src/native...
File pkg/compiler/lib/src/native/resolver.dart (right):

https://codereview.chromium.org/2836993003/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:347: /// Interface for computing all
native classes in a set of libraries.
nit: 

" Interface for computing ..."

=>

" Determines ... "

https://codereview.chromium.org/2836993003/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:450: class
ResolutionNativeClassResolver extends NativeClassResolverImpl {
I don't mind resolver, but another idea: rename "resolver" to "finder"?

NativeClassFinder -> finds native classes in libraries
BaseNativeClassFinder
ResolutionNativeClassFinder

https://codereview.chromium.org/2836993003/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:472: // on only one native class.
don't we need this validation with Kernel too?

https://codereview.chromium.org/2836993003/diff/1/tests/compiler/dart2js/kern...
File tests/compiler/dart2js/kernel/closed_world2_test.dart (right):

https://codereview.chromium.org/2836993003/diff/1/tests/compiler/dart2js/kern...
tests/compiler/dart2js/kernel/closed_world2_test.dart:175: NativeClassResolver
nativeClassResolver = new NativeClassResolverImpl(
should we start using the strategy here?

[Johnni Winther, 2017-04-26 07:40:19]

https://codereview.chromium.org/2836993003/diff/1/pkg/compiler/lib/src/native...
File pkg/compiler/lib/src/native/resolver.dart (right):

https://codereview.chromium.org/2836993003/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:347: /// Interface for computing all
native classes in a set of libraries.
On 2017/04/25 19:45:53, Siggi Cherem (dart-lang) wrote:
> nit: 
> 
> " Interface for computing ..."
> 
> =>
> 
> " Determines ... "

Done.

https://codereview.chromium.org/2836993003/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:450: class
ResolutionNativeClassResolver extends NativeClassResolverImpl {
On 2017/04/25 19:45:53, Siggi Cherem (dart-lang) wrote:
> I don't mind resolver, but another idea: rename "resolver" to "finder"?
> 
> NativeClassFinder -> finds native classes in libraries
> BaseNativeClassFinder
> ResolutionNativeClassFinder

Done.

https://codereview.chromium.org/2836993003/diff/1/pkg/compiler/lib/src/native...
pkg/compiler/lib/src/native/resolver.dart:472: // on only one native class.
On 2017/04/25 19:45:53, Siggi Cherem (dart-lang) wrote:
> don't we need this validation with Kernel too?

We do. Moved this to BaseNativeClassFinder._processNativeClass and added a
super-call here.

https://codereview.chromium.org/2836993003/diff/1/tests/compiler/dart2js/kern...
File tests/compiler/dart2js/kernel/closed_world2_test.dart (right):

https://codereview.chromium.org/2836993003/diff/1/tests/compiler/dart2js/kern...
tests/compiler/dart2js/kernel/closed_world2_test.dart:175: NativeClassResolver
nativeClassResolver = new NativeClassResolverImpl(
On 2017/04/25 19:45:53, Siggi Cherem (dart-lang) wrote:
> should we start using the strategy here?

Yes.

[Johnni Winther, 2017-04-26 07:50:29]

Description was changed from

==========
Handle instantiation of native classes in closed_world2_test
==========

to

==========
Handle instantiation of native classes in closed_world2_test

R=sigmund@google.com

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

[Johnni Winther, 2017-04-26 07:50:30]

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