dart2js: add common names to mangledGlobalNames in new emitter.

dart2js: add common names to mangledGlobalNames in new emitter.

R=floitsch@google.com

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

[zarah, 2015-02-04 16:05:52]

zarah@google.com changed reviewers:
+ floitsch@google.com, herhut@google.com

[floitsch, 2015-02-04 16:18:28]

LGTM.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
File pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart (right):

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:238:
[compiler.intClass, compiler.doubleClass, compiler.numClass,
Add comment why this is needed/wanted.

Update the test that failed and explain that this is not required but wanted
behavior.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:239:
compiler.stringClass, compiler.boolClass, compiler.nullClass,
indent by 1.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:240:
compiler.listClass].forEach((cls) =>
make this a "{}" function body.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:240:
compiler.listClass].forEach((cls) =>
Don't use "cls". "Cls" is only used for "Class" instances. ClassElements are
always called element or classElement.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:240:
compiler.listClass].forEach((cls) =>
Split it into smaller pieces.
List<ClassElement> nativeClassesNeedingUnmangledName =
  [compiler.stringClass, ...];

nativeClassesNeedingUnmangledName.forEach(...)

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:245: new
js.ObjectInitializer(names, isOneLiner: false));
I would have kept it as a one-liner.

[zarah, 2015-02-05 08:19:41]

New patchsets have been uploaded after l-g-t-m from floitsch@google.com

[floitsch, 2015-02-05 10:55:35]

Still LGTM.

https://codereview.chromium.org/897013003/diff/20001/tests/language/cyclic_ty...
File tests/language/cyclic_type_test.dart (right):

https://codereview.chromium.org/897013003/diff/20001/tests/language/cyclic_ty...
tests/language/cyclic_type_test.dart:31: // In the tests below we test that we
get "int" when calling toString() on the
(same is true for "bool")

https://codereview.chromium.org/897013003/diff/20001/tests/language/cyclic_ty...
tests/language/cyclic_type_test.dart:32: // int type. This is not required
behaviour. However, we want to keep the
tiny tiny nit: behavior.
(we generally use US english)

[zarah, 2015-02-06 08:47:07]

New patchsets have been uploaded after l-g-t-m from floitsch@google.com

[zarah, 2015-02-06 08:52:23]

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

[zarah, 2015-02-06 08:53:12]

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
File pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart (right):

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:238:
[compiler.intClass, compiler.doubleClass, compiler.numClass,
On 2015/02/04 16:18:28, floitsch wrote:
> Add comment why this is needed/wanted.
> 
> Update the test that failed and explain that this is not required but wanted
> behavior.

Done.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:239:
compiler.stringClass, compiler.boolClass, compiler.nullClass,
On 2015/02/04 16:18:28, floitsch wrote:
> indent by 1.

Done.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:240:
compiler.listClass].forEach((cls) =>
On 2015/02/04 16:18:28, floitsch wrote:
> Don't use "cls". "Cls" is only used for "Class" instances. ClassElements are
> always called element or classElement.

Done.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:240:
compiler.listClass].forEach((cls) =>
On 2015/02/04 16:18:28, floitsch wrote:
> Split it into smaller pieces.
> List<ClassElement> nativeClassesNeedingUnmangledName =
>   [compiler.stringClass, ...];
> 
> nativeClassesNeedingUnmangledName.forEach(...)

Done.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:240:
compiler.listClass].forEach((cls) =>
On 2015/02/04 16:18:28, floitsch wrote:
> make this a "{}" function body.

Done.

https://codereview.chromium.org/897013003/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:245: new
js.ObjectInitializer(names, isOneLiner: false));
On 2015/02/04 16:18:28, floitsch wrote:
> I would have kept it as a one-liner.

Done.

https://codereview.chromium.org/897013003/diff/20001/tests/language/cyclic_ty...
File tests/language/cyclic_type_test.dart (right):

https://codereview.chromium.org/897013003/diff/20001/tests/language/cyclic_ty...
tests/language/cyclic_type_test.dart:31: // In the tests below we test that we
get "int" when calling toString() on the
On 2015/02/05 10:55:35, floitsch wrote:
> (same is true for "bool")

Done.

https://codereview.chromium.org/897013003/diff/20001/tests/language/cyclic_ty...
tests/language/cyclic_type_test.dart:32: // int type. This is not required
behaviour. However, we want to keep the
On 2015/02/05 10:55:35, floitsch wrote:
> tiny tiny nit: behavior.
> (we generally use US english)

Done.