Implement more of dart:mirrors

Implement more of dart:mirrors

This unblocks angular.

See #199

R=jmesserly@google.com

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

[vsm, 2015-07-24 15:01:32]

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

[vsm, 2015-07-24 15:01:38]

https://codereview.chromium.org/1252953003/diff/1/karma.conf.js
File karma.conf.js (right):

https://codereview.chromium.org/1252953003/diff/1/karma.conf.js#newcode56
karma.conf.js:56: logLevel: config.LOG_INFO,
This is unrelated - just suppressing some of the debug on normal test runs.

https://codereview.chromium.org/1252953003/diff/1/lib/runtime/_errors.js
File lib/runtime/_errors.js (right):

https://codereview.chromium.org/1252953003/diff/1/lib/runtime/_errors.js#newc...
lib/runtime/_errors.js:19: operations.throw(new core.NoSuchMethodError(obj,
name, pArgs, nArgs, extras));
Fixing these by hand along the lines of previous cl.

[Jennifer Messerly, 2015-07-24 16:28:28]

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

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1389: JS.ArrayInitializer
_emitTypeNames(List<DartType> types,
thought for future:
should we only emit mirrors under certain conditions, like when a flag is
passed?
anyway I filed https://github.com/dart-lang/dev_compiler/issues/268 for that

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1393: var parameter = parameters != null ?
parameters[i] : null;
parameters[i] will never be null right? i'd be tempted to restructure this a
bit:

List metadata = [];
if (parameters != null) {
  metadata = parameters[i].metadata; // see my comment below, this is an
analyzer API tweak  
}
var typeName = _emitTypeName(types[i]);
var value = typeName;
if (metadata.isNotEmpty) {
   ...
}
...

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1397: : (parameter as
DefaultFormalParameter).parameter.metadata
hmm, this might be good feedback to analyzer. Would a forwarder help?
I'm happy to send a CL over to them. We're going to depend on 0.26-alpha soon
anyway (needed a bug fix to ResolutionCopier for AwaitExpression)

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1424: List<FormalParameter> parameters =
should this be:

    var parameters = parameterList?.parameters;

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1440: var oa = _emitTypeNames(optionalTypes,
parameters != null
ditto:

    parameters?.sublist(parameterTypes.length)


FWIW, I'm considering a tool that will auto fix these. But might be nice to have
a few landed by hand, so we work out any kinks it causes :)

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1631: // code = 'dart.$DSEND(#, #, #)';
remove commented code?

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
File tool/input_sdk/private/js_mirrors.dart (right):

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
tool/input_sdk/private/js_mirrors.dart:51: ClassMirror get type => throw new
UnimplementedError("ClassMirror.type unimplemented");
long lines

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
tool/input_sdk/private/js_mirrors.dart:102: var fn = JS('List',
'#[dart.metadata]', _cls);
fwiw, we could fix our "type parser" for "JS" to understand generics...

I've wanted to fix it for a while because it's *so* dumb right now (doesn't even
understand how to look up the type name from anywhere but core!)

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
tool/input_sdk/private/js_mirrors.dart:131: InstanceMirror getField(Symbol
fieldName) => throw new UnimplementedError("ClassMirror.getField
unimplemented");
here too

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
tool/input_sdk/private/js_mirrors.dart:160: final bool hasReflectedType = true;
make this a getter? (less storage needed on the instance)

https://codereview.chromium.org/1252953003/diff/1/tool/sdk_expected_errors.txt
File tool/sdk_expected_errors.txt (right):

https://codereview.chromium.org/1252953003/diff/1/tool/sdk_expected_errors.tx...
tool/sdk_expected_errors.txt:378: severe: [AnalyzerMessage] 'fn' is not a method
(dart:_js_mirrors, line 105, col 41)
fix these?

[Jennifer Messerly, 2015-07-24 16:30:02]

fyi, I'm going to push a branch to test a null aware op ... it works in VM but
not sure everywhere yet :)

[Jennifer Messerly, 2015-07-24 16:40:00]

Quick report on ?? and ?.

+ works in VM, analyzer, all of our tests pass
~ dart_style requires upgrade to 2.0-rc, but we needed that anyway very soon,
because we need to roll forward our analyzer. (I'll send a CL for it)
~ IntelliJ requires update to latest EAP, but works!

Anyway, no need to change your CL if you don't want, but IMO we should start
using it ... gonna send a test CL to travis :)

[vsm, 2015-08-07 17:07:33]

thanks - ptal

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

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1389: JS.ArrayInitializer
_emitTypeNames(List<DartType> types,
On 2015/07/24 16:28:28, John Messerly wrote:
> thought for future:
> should we only emit mirrors under certain conditions, like when a flag is
> passed?
> anyway I filed https://github.com/dart-lang/dev_compiler/issues/268 for that

Makes sense.  Added a comment.

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1393: var parameter = parameters != null ?
parameters[i] : null;
On 2015/07/24 16:28:28, John Messerly wrote:
> parameters[i] will never be null right? i'd be tempted to restructure this a
> bit:
> 
> List metadata = [];
> if (parameters != null) {
>   metadata = parameters[i].metadata; // see my comment below, this is an
> analyzer API tweak  
> }
> var typeName = _emitTypeName(types[i]);
> var value = typeName;
> if (metadata.isNotEmpty) {
>    ...
> }
> ...

Done.

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1397: : (parameter as
DefaultFormalParameter).parameter.metadata
On 2015/07/24 16:28:28, John Messerly wrote:
> hmm, this might be good feedback to analyzer. Would a forwarder help?
> I'm happy to send a CL over to them. We're going to depend on 0.26-alpha soon
> anyway (needed a bug fix to ResolutionCopier for AwaitExpression)

Factored it out a bit to a helper here.  A forwarder sounds good - you still
hacking that code?

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1424: List<FormalParameter> parameters =
On 2015/07/24 16:28:28, John Messerly wrote:
> should this be:
> 
>     var parameters = parameterList?.parameters;

Done.

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1440: var oa = _emitTypeNames(optionalTypes,
parameters != null
On 2015/07/24 16:28:28, John Messerly wrote:
> ditto:
> 
>     parameters?.sublist(parameterTypes.length)
> 
> 
> FWIW, I'm considering a tool that will auto fix these. But might be nice to
have
> a few landed by hand, so we work out any kinks it causes :)

Done.

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1631: // code = 'dart.$DSEND(#, #, #)';
On 2015/07/24 16:28:28, John Messerly wrote:
> remove commented code?

Done.

https://codereview.chromium.org/1252953003/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:1631: // code = 'dart.$DSEND(#, #, #)';
On 2015/07/24 16:28:28, John Messerly wrote:
> remove commented code?

Done.

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
File tool/input_sdk/private/js_mirrors.dart (right):

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
tool/input_sdk/private/js_mirrors.dart:51: ClassMirror get type => throw new
UnimplementedError("ClassMirror.type unimplemented");
On 2015/07/24 16:28:28, John Messerly wrote:
> long lines

Done.

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
tool/input_sdk/private/js_mirrors.dart:102: var fn = JS('List',
'#[dart.metadata]', _cls);
On 2015/07/24 16:28:28, John Messerly wrote:
> fwiw, we could fix our "type parser" for "JS" to understand generics...
> 
> I've wanted to fix it for a while because it's *so* dumb right now (doesn't
even
> understand how to look up the type name from anywhere but core!)

Up to you.  We don't really use it so much I guess - unless we make a version of
this available to users.

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
tool/input_sdk/private/js_mirrors.dart:131: InstanceMirror getField(Symbol
fieldName) => throw new UnimplementedError("ClassMirror.getField
unimplemented");
On 2015/07/24 16:28:28, John Messerly wrote:
> here too

Done.

https://codereview.chromium.org/1252953003/diff/1/tool/input_sdk/private/js_m...
tool/input_sdk/private/js_mirrors.dart:160: final bool hasReflectedType = true;
On 2015/07/24 16:28:28, John Messerly wrote:
> make this a getter? (less storage needed on the instance)

Could be ... but won't a constant for long ... so maybe not worth it.

https://codereview.chromium.org/1252953003/diff/1/tool/sdk_expected_errors.txt
File tool/sdk_expected_errors.txt (right):

https://codereview.chromium.org/1252953003/diff/1/tool/sdk_expected_errors.tx...
tool/sdk_expected_errors.txt:378: severe: [AnalyzerMessage] 'fn' is not a method
(dart:_js_mirrors, line 105, col 41)
On 2015/07/24 16:28:28, John Messerly wrote:
> fix these?

Done.

[vsm, 2015-08-07 18:20:33]

https://codereview.chromium.org/1252953003/diff/40001/lib/runtime/dart_utils.js
File lib/runtime/dart_utils.js (right):

https://codereview.chromium.org/1252953003/diff/40001/lib/runtime/dart_utils....
lib/runtime/dart_utils.js:116: var hideMap = new Set(hide);
John - does this look right?  I was hitting an error here where hide was an
Array.  Can it ever be anything else?

[Jennifer Messerly, 2015-08-07 18:52:59]

LGTM!

https://codereview.chromium.org/1252953003/diff/40001/lib/runtime/dart_utils.js
File lib/runtime/dart_utils.js (right):

https://codereview.chromium.org/1252953003/diff/40001/lib/runtime/dart_utils....
lib/runtime/dart_utils.js:116: var hideMap = new Set(hide);
On 2015/08/07 18:20:33, vsm wrote:
> John - does this look right?  I was hitting an error here where hide was an
> Array.  Can it ever be anything else?

oh my gosh, good catch, it was supposed to be Set. Ugh, I guess none of the
tests caught this either. Thanks for fixing!

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

https://codereview.chromium.org/1252953003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1484: // TODO(vsm):
forgot comment?

[vsm, 2015-08-07 19:38:31]

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

https://codereview.chromium.org/1252953003/diff/40001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1484: // TODO(vsm):
On 2015/08/07 18:52:59, John Messerly wrote:
> forgot comment?

Doh - thanks - this was the comment I "moved" to line 1498.  Removed.

[vsm, 2015-08-07 19:40:44]

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