optimize self-references in generic type definitions

optimize self-references in generic type definitions

fixes #556

R=jmesserly@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/e1aa512528e1a942722fc3ca30a2edb18e8fa9bd

[Harry Terkelsen, 2016-05-09 18:03:38]

het@google.com changed reviewers:
+ jmesserly@google.com, vsm@google.com

[Harry Terkelsen, 2016-05-09 18:04:19]

https://codereview.chromium.org/1958193002/diff/1/lib/src/compiler/code_gener...
File lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/1958193002/diff/1/lib/src/compiler/code_gener...
lib/src/compiler/code_generator.dart:2232: ClassElement subClass,
had to add 2 named args just for this one case :( not sure if there's a nicer
way?

[Jennifer Messerly, 2016-05-09 18:16:36]

A suggestion but this LGTM as it is too :)

https://codereview.chromium.org/1958193002/diff/1/lib/src/compiler/code_gener...
File lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/1958193002/diff/1/lib/src/compiler/code_gener...
lib/src/compiler/code_generator.dart:2232: ClassElement subClass,
On 2016/05/09 18:04:19, Harry Terkelsen wrote:
> had to add 2 named args just for this one case :( not sure if there's a nicer
> way?

Hmmm. You could pass it down as a function from `(DartType) -> JS.Expression`
that returns null if it doesn't need to replace. Not sure that's a lot better
though :)

Another idea is to do the classic recursive function split, where you have an
outer function and an inner function, with the outer one taking fewer
parameters. So _emitType and _emitTypeInternal

BTW at some point we were thinking renaming `_emitTypeName` to `_emitType` would
be good? Since "TypeName" sounds like the AST node TypeName, but the parameter
is a DartType not a TypeName.

[Harry Terkelsen, 2016-05-09 18:41:03]

On 2016/05/09 18:16:36, John Messerly wrote:
> A suggestion but this LGTM as it is too :)
> 
>
https://codereview.chromium.org/1958193002/diff/1/lib/src/compiler/code_gener...
> File lib/src/compiler/code_generator.dart (right):
> 
>
https://codereview.chromium.org/1958193002/diff/1/lib/src/compiler/code_gener...
> lib/src/compiler/code_generator.dart:2232: ClassElement subClass,
> On 2016/05/09 18:04:19, Harry Terkelsen wrote:
> > had to add 2 named args just for this one case :( not sure if there's a
nicer
> > way?
> 
> Hmmm. You could pass it down as a function from `(DartType) -> JS.Expression`
> that returns null if it doesn't need to replace. Not sure that's a lot better
> though :)
> 
> Another idea is to do the classic recursive function split, where you have an
> outer function and an inner function, with the outer one taking fewer
> parameters. So _emitType and _emitTypeInternal
> 
> BTW at some point we were thinking renaming `_emitTypeName` to `_emitType`
would
> be good? Since "TypeName" sounds like the AST node TypeName, but the parameter
> is a DartType not a TypeName.

I changed it to _emitType in this CL, I agree that's better

[Harry Terkelsen, 2016-05-09 18:41:04]

On 2016/05/09 18:16:36, John Messerly wrote:
> A suggestion but this LGTM as it is too :)
> 
>
https://codereview.chromium.org/1958193002/diff/1/lib/src/compiler/code_gener...
> File lib/src/compiler/code_generator.dart (right):
> 
>
https://codereview.chromium.org/1958193002/diff/1/lib/src/compiler/code_gener...
> lib/src/compiler/code_generator.dart:2232: ClassElement subClass,
> On 2016/05/09 18:04:19, Harry Terkelsen wrote:
> > had to add 2 named args just for this one case :( not sure if there's a
nicer
> > way?
> 
> Hmmm. You could pass it down as a function from `(DartType) -> JS.Expression`
> that returns null if it doesn't need to replace. Not sure that's a lot better
> though :)
> 
> Another idea is to do the classic recursive function split, where you have an
> outer function and an inner function, with the outer one taking fewer
> parameters. So _emitType and _emitTypeInternal
> 
> BTW at some point we were thinking renaming `_emitTypeName` to `_emitType`
would
> be good? Since "TypeName" sounds like the AST node TypeName, but the parameter
> is a DartType not a TypeName.

I changed it to _emitType in this CL, I agree that's better

[Harry Terkelsen, 2016-05-09 18:43:00]

Description was changed from

==========
optimize self-references in generic type definitions

fixes #556
==========

to

==========
optimize self-references in generic type definitions

fixes #556

R=jmesserly@google.com

Committed:
https://github.com/dart-lang/dev_compiler/commit/e1aa512528e1a942722fc3ca30a2...
==========

[Harry Terkelsen, 2016-05-09 18:43:01]

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