Compute NativeBasicData for KernelWorldBuilder

Compute NativeBasicData for KernelWorldBuilder

This computes the native classes using metadata from the KernelWorldBuilder.
This will be merge with the AnnotationProcessor from https://codereview.chromium.org/2824123003/

R=sigmund@google.com

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

[Johnni Winther, 2017-04-19 12:05:21]

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

[Siggi Cherem (dart-lang), 2017-04-19 16:05:23]

https://codereview.chromium.org/2824423002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/native_basic_data.dart (right):

https://codereview.chromium.org/2824423002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/native_basic_data.dart:20: in
worldBuilder._getClassMetadata(cls)) {
I might be missing sometheing, but isn't this a type mismatch here? (`cls` is an
entity, but _getClassMetadata expects a `KClassEnv`)?

I expect we want to fix _getClassMetadata, right? (we want to keep KEnv* private
to this library, correct?) If so, can we make them private classes too?

https://codereview.chromium.org/2824423002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/native_basic_data.dart:22:
worldBuilder.constantEnvironment.getConstantValue(annotation);
ah - this might answer part of my question from my other CL - do we need to have
a world builder in order to get a constant environment? Does that mean that we
can't really do much without a world builder from early stages of the compiler?

https://codereview.chromium.org/2824423002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/native_basic_data.dart:38: return
builder.close(worldBuilder.elementEnvironment);
use directly the local elementEnvironment

[Johnni Winther, 2017-04-20 08:05:25]

PTAL

https://codereview.chromium.org/2824423002/diff/1/pkg/compiler/lib/src/kernel...
File pkg/compiler/lib/src/kernel/native_basic_data.dart (right):

https://codereview.chromium.org/2824423002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/native_basic_data.dart:20: in
worldBuilder._getClassMetadata(cls)) {
On 2017/04/19 16:05:22, Siggi Cherem (dart-lang) wrote:
> I might be missing sometheing, but isn't this a type mismatch here? (`cls` is
an
> entity, but _getClassMetadata expects a `KClassEnv`)?

_getClassMetadata expects a KClass but uses _classEnvs to compute its metadata.

> 
> I expect we want to fix _getClassMetadata, right? (we want to keep KEnv*
private
> to this library, correct?) If so, can we make them private classes too?

Will do in a later CL.

https://codereview.chromium.org/2824423002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/native_basic_data.dart:22:
worldBuilder.constantEnvironment.getConstantValue(annotation);
On 2017/04/19 16:05:22, Siggi Cherem (dart-lang) wrote:
> ah - this might answer part of my question from my other CL - do we need to
have
> a world builder in order to get a constant environment? Does that mean that we
> can't really do much without a world builder from early stages of the
compiler?

We need the KernelWorldBuilder from the get-go. That's why it needs to be in the
'FrontStrategy' for kernel.

https://codereview.chromium.org/2824423002/diff/1/pkg/compiler/lib/src/kernel...
pkg/compiler/lib/src/kernel/native_basic_data.dart:38: return
builder.close(worldBuilder.elementEnvironment);
On 2017/04/19 16:05:22, Siggi Cherem (dart-lang) wrote:
> use directly the local elementEnvironment

Done.

[Johnni Winther, 2017-04-21 07:23:03]

PING

[Siggi Cherem (dart-lang), 2017-04-21 16:29:40]

lgtm

https://codereview.chromium.org/2824423002/diff/40001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/native_basic_data.dart (right):

https://codereview.chromium.org/2824423002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/native_basic_data.dart:8: // TODO(johnnniwinther):
Move this to the AnnotationProcessor when it has been
update TODO? I'm guessing this function will entirely go away, since the
compiler will create the processor directly and call it like we already do?

Maybe we should move this function to the unit test where we use it?

https://codereview.chromium.org/2824423002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/native_basic_data.dart:37: in
worldBuilder._getClassMetadata(cls)) {
add TODO:
 - make getClassMetadata public (with our plan of checking in unit tests that we
don't abuse it)
 - move this processor out of this library

[Johnni Winther, 2017-04-24 06:50:07]

https://codereview.chromium.org/2824423002/diff/40001/pkg/compiler/lib/src/ke...
File pkg/compiler/lib/src/kernel/native_basic_data.dart (right):

https://codereview.chromium.org/2824423002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/native_basic_data.dart:8: // TODO(johnnniwinther):
Move this to the AnnotationProcessor when it has been
On 2017/04/21 16:29:39, Siggi Cherem (dart-lang) wrote:
> update TODO? I'm guessing this function will entirely go away, since the
> compiler will create the processor directly and call it like we already do?
> 
> Maybe we should move this function to the unit test where we use it?

Done.

https://codereview.chromium.org/2824423002/diff/40001/pkg/compiler/lib/src/ke...
pkg/compiler/lib/src/kernel/native_basic_data.dart:37: in
worldBuilder._getClassMetadata(cls)) {
On 2017/04/21 16:29:39, Siggi Cherem (dart-lang) wrote:
> add TODO:
>  - make getClassMetadata public (with our plan of checking in unit tests that
we
> don't abuse it)
>  - move this processor out of this library

Done.

[Johnni Winther, 2017-04-24 06:56:40]

Description was changed from

==========
Compute NativeBasicData for KernelWorldBuilder

This computes the native classes using metadata from the KernelWorldBuilder.
This will be merge with the AnnotationProcessor from
https://codereview.chromium.org/2824123003/
==========

to

==========
Compute NativeBasicData for KernelWorldBuilder

This computes the native classes using metadata from the KernelWorldBuilder.
This will be merge with the AnnotationProcessor from
https://codereview.chromium.org/2824123003/

R=sigmund@google.com

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

[Johnni Winther, 2017-04-24 06:56:41]

Committed patchset #4 (id:60001) manually as
1c0684ceabf4e6cd9df3f0c3cdc8cd5918236248 (presubmit successful).