Fixes #178

Fixes #178

Expand out constructor call if needed.

R=jmesserly@google.com

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

[vsm, 2015-05-11 14:50:02]

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

[Jennifer Messerly, 2015-05-11 16:35:08]

a few suggestions otherwise looks good

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:339: _currentClass = type;
suggestion: maybe save the Element here, not the type. Will make it easier to
compare later.

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:353: _classHeritage(node),
_emitClassMethods(node, ctors, fields));
I think after this change, calls to static methods from inside the class might
generate as:

let Foo$ = dart.generic((E) => {
  class Foo {
    static foo() { ... }
    static bar() { Foo(dart.dynamic).foo(); }
  }
});

(ditto for static field references)


Perf cost, but maybe not a big deal for now?
Anyway, just something I've been wondering about. Possibly worth a test case.

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1250: var isCurrentClass =
could this just be:

    var isCurrentClass = type.element == _currentClass?

(with suggestion above to use the ClassElement)

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1252: if (args.isNotEmpty && isCurrentClass ||
maybe split these into two cases:

if (args.any((a) => a != types.dynamicType)) {
  ... original code ...
} else if (args.isNotEmpty && isCurrentClass) {
  // When creating a `new S<dynamic>` we try and use the raw form `new S()`,
  // but this does not work if we're inside the same class, because `S` refers
  // to the current S<T> we are generating.
  name = '$name\$';
  typeArgs = [];
}


The benefit is the second case will generate using the shorter form `Map$()`
instead of `Map$(dart.dynamic, dart.dynamic)`  [those two forms are equivalent]

[vsm, 2015-05-11 17:51:09]

PTAL

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:339: _currentClass = type;
On 2015/05/11 16:35:08, John Messerly wrote:
> suggestion: maybe save the Element here, not the type. Will make it easier to
> compare later.

Done.

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:353: _classHeritage(node),
_emitClassMethods(node, ctors, fields));
On 2015/05/11 16:35:08, John Messerly wrote:
> I think after this change, calls to static methods from inside the class might
> generate as:
> 
> let Foo$ = dart.generic((E) => {
>   class Foo {
>     static foo() { ... }
>     static bar() { Foo(dart.dynamic).foo(); }
>   }
> });
> 
> (ditto for static field references)
> 
> 
> Perf cost, but maybe not a big deal for now?
> Anyway, just something I've been wondering about. Possibly worth a test case.

Added a test case to track this.

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1250: var isCurrentClass =
On 2015/05/11 16:35:08, John Messerly wrote:
> could this just be:
> 
>     var isCurrentClass = type.element == _currentClass?
> 
> (with suggestion above to use the ClassElement)
> 

Done.

https://codereview.chromium.org/1135543003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1252: if (args.isNotEmpty && isCurrentClass ||
On 2015/05/11 16:35:08, John Messerly wrote:
> maybe split these into two cases:
> 
> if (args.any((a) => a != types.dynamicType)) {
>   ... original code ...
> } else if (args.isNotEmpty && isCurrentClass) {
>   // When creating a `new S<dynamic>` we try and use the raw form `new S()`,
>   // but this does not work if we're inside the same class, because `S` refers
>   // to the current S<T> we are generating.
>   name = '$name\$';
>   typeArgs = [];
> }
> 
> 
> The benefit is the second case will generate using the shorter form `Map$()`
> instead of `Map$(dart.dynamic, dart.dynamic)`  [those two forms are
equivalent]
> 
> 

Ahh, nice.  Done!

[Jennifer Messerly, 2015-05-11 17:52:56]

lgtm!

https://codereview.chromium.org/1135543003/diff/20001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1135543003/diff/20001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1254: // When creating a `new S<dynamic>` we try
and use the raw form `new S()`,
maybe a long line?

[vsm, 2015-05-11 17:56:03]

https://codereview.chromium.org/1135543003/diff/20001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1135543003/diff/20001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1254: // When creating a `new S<dynamic>` we try
and use the raw form `new S()`,
On 2015/05/11 17:52:56, John Messerly wrote:
> maybe a long line?

Done.

[vsm, 2015-05-11 17:56:19]

Committed patchset #3 (id:40001) manually as
8c8828a64104eaf64da79e097ff265208937d714 (presubmit successful).