dart2js: Add support for static fields to new emitter.

dart2js: Add support for static fields to new emitter.

R=kasperl@google.com

Committed: https://code.google.com/p/dart/source/detail?r=41160

[floitsch, 2014-10-16 14:27:54]

floitsch@google.com changed reviewers:
+ kasperl@google.com

[floitsch, 2014-10-16 14:27:55]

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_emitter/model.dart (right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/model.dart:106: final
Holder holder;
The holder for static fields in the "isolate" object "$".
I'm not sure if we need to store it here all the time...

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_emitter/program_builder.dart
(right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/program_builder.dart:239:
void _registerConstants(OutputUnit outputUnit,
I think I can simplify this in a later CL: turns out that we don't use the
_constant mapping, since we just generate the code to initialize the statics...

[kasperl, 2014-10-17 06:47:04]

LGTM.

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_emitter/model.dart (right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/model.dart:106: final
Holder holder;
On 2014/10/16 14:27:55, floitsch wrote:
> The holder for static fields in the "isolate" object "$".
> I'm not sure if we need to store it here all the time...

Maybe add this as a comment in the code?

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File
sdk/lib/_internal/compiler/implementation/js_emitter/new_emitter/model_emitter.dart
(right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/new_emitter/model_emitter.dart:329:
// Initialize static non-final fields constants.
Remove constants?

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_emitter/program_builder.dart
(right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/program_builder.dart:148:
return Elements.sortedByPosition(staticNonFinalFields).map
Maybe turn the closure passed to map here into a proper method? It's complex
enough to warrant its own name.

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_emitter/registry.dart (right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/registry.dart:25:
Fragment.deferred(this.outputUnit, this.name);
Assert that name isn't empty?

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/registry.dart:27: bool get
isMainFragment => name == "";
I guess this is more convenient than asking the registry? Probably.

[floitsch, 2014-10-17 09:25:57]

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_emitter/model.dart (right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/model.dart:106: final
Holder holder;
On 2014/10/17 06:47:04, kasperl wrote:
> On 2014/10/16 14:27:55, floitsch wrote:
> > The holder for static fields in the "isolate" object "$".
> > I'm not sure if we need to store it here all the time...
> 
> Maybe add this as a comment in the code?

Done.

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File
sdk/lib/_internal/compiler/implementation/js_emitter/new_emitter/model_emitter.dart
(right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/new_emitter/model_emitter.dart:329:
// Initialize static non-final fields constants.
On 2014/10/17 06:47:04, kasperl wrote:
> Remove constants?

doh.
done.

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_emitter/program_builder.dart
(right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/program_builder.dart:148:
return Elements.sortedByPosition(staticNonFinalFields).map
On 2014/10/17 06:47:04, kasperl wrote:
> Maybe turn the closure passed to map here into a proper method? It's complex
> enough to warrant its own name.

Done.

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/js_emitter/registry.dart (right):

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/registry.dart:25:
Fragment.deferred(this.outputUnit, this.name);
On 2014/10/17 06:47:04, kasperl wrote:
> Assert that name isn't empty?

Done.

https://codereview.chromium.org/661783002/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/js_emitter/registry.dart:27: bool get
isMainFragment => name == "";
On 2014/10/17 06:47:04, kasperl wrote:
> I guess this is more convenient than asking the registry? Probably.

Since we already have the field in the registry I changed it.
done.

[floitsch, 2014-10-17 09:26:23]

Committed patchset #2 (id:20001) manually as 41160 (presubmit successful).