dart2js: Reuse names for constructor bodes.

dart2js: Reuse names for constructor bodes.

BUG=
R=sra@google.com

Committed: https://github.com/dart-lang/sdk/commit/7ea6facff6f130531f9e064c43a4bf5f82574229

[herhut, 2015-06-30 19:58:14]

herhut@google.com changed reviewers:
+ sra@google.com

[herhut, 2015-06-30 19:58:15]

Here is a first idea on how to reuse names for constructor bodies. 

Just FYI, needs more testing.

[herhut, 2015-07-01 12:24:32]

PTAL.

[sra1, 2015-07-01 16:15:47]

lgtm with comments.

How well does it work?

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/minify_namer.dart (right):

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/minify_namer.dart:259: class
_ConstructorBodyNamingScope {
Add a comment saying what this is doing and why it is OK.

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/minify_namer.dart:286: factory
_ConstructorBodyNamingScope(ClassElement cls, Map<ClassElement,
break between parameters before breaking within parameters.

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/minify_namer.dart:320: return
_disambiguateInternalMember(method, null);
remove dead code.
I was trying to understand how this line worked :-)

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/namer.dart (right):

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/namer.dart:929: /// by element at the same time,
as two different names would be returned.
Can we have an assert for that?

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/namer.dart:937: jsAst.Name
_disambiguateMemberByKey(String key,
fits on one line.

[herhut, 2015-07-02 09:03:00]

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/minify_namer.dart (right):

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/minify_namer.dart:259: class
_ConstructorBodyNamingScope {
On 2015/07/01 16:15:47, sra1 wrote:
> Add a comment saying what this is doing and why it is OK.

Done.

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/minify_namer.dart:286: factory
_ConstructorBodyNamingScope(ClassElement cls, Map<ClassElement,
On 2015/07/01 16:15:47, sra1 wrote:
> break between parameters before breaking within parameters.

Done.

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/minify_namer.dart:320: return
_disambiguateInternalMember(method, null);
On 2015/07/01 16:15:47, sra1 wrote:
> remove dead code.
> I was trying to understand how this line worked :-)

Sorry. Done.

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js_backend/namer.dart (right):

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/namer.dart:929: /// by element at the same time,
as two different names would be returned.
On 2015/07/01 16:15:47, sra1 wrote:
> Can we have an assert for that?

No, because there is no well defined relation between elements and keys. So I
cannot detect this situation.

https://codereview.chromium.org/1215223002/diff/30002/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js_backend/namer.dart:937: jsAst.Name
_disambiguateMemberByKey(String key,
On 2015/07/01 16:15:47, sra1 wrote:
> fits on one line.

Done.

[herhut, 2015-07-02 09:41:14]

Committed patchset #5 (id:70001) manually as
7ea6facff6f130531f9e064c43a4bf5f82574229 (presubmit successful).