Support reflection on recursive upper bounds of type variables.

Support reflection on recursive upper bounds of type variables.

R=johnniwinther@google.com

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

[ahe, 2013-10-14 13:43:57]

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:186: int bound;
Needs to be private, I think.

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:189: JsTypeVariableMirror(Symbol
simpleName, this.bound, this.owner)
JsTypeVariableMirror(TypeVariable tv, this.owner)
    : this.tv = tv,
      super(tv.name);

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:831: int parsedIndex = int.parse(typeArg,
onError: (_) => -1);
Document how this is parsed, perhaps above in the documentation of _typeArgs.

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:847: StringBuffer currentTypeArg = new
StringBuffer();
Probably more efficient to use string concatenation (because of how we compile
to JavaScript).

[zarah, 2013-10-21 07:25:55]

PTAL.

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:186: int bound;
On 2013/10/14 13:43:58, ahe wrote:
> Needs to be private, I think.

Done.

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:189: JsTypeVariableMirror(Symbol
simpleName, this.bound, this.owner)
On 2013/10/14 13:43:58, ahe wrote:
> JsTypeVariableMirror(TypeVariable tv, this.owner)
>     : this.tv = tv,
>       super(tv.name);

Changed in previous cl.

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:831: int parsedIndex = int.parse(typeArg,
onError: (_) => -1);
On 2013/10/14 13:43:58, ahe wrote:
> Document how this is parsed, perhaps above in the documentation of _typeArgs.

Done.

https://codereview.chromium.org/27064004/diff/1/sdk/lib/_internal/lib/js_mirr...
sdk/lib/_internal/lib/js_mirrors.dart:847: StringBuffer currentTypeArg = new
StringBuffer();
On 2013/10/14 13:43:58, ahe wrote:
> Probably more efficient to use string concatenation (because of how we compile
> to JavaScript).

Done.

[Johnni Winther, 2013-10-22 06:54:16]

lgtm

https://codereview.chromium.org/27064004/diff/6001/sdk/lib/_internal/lib/js_m...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/27064004/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:846:
result.add(reflectClassByMangledName(typeArg.trim()));
Is it really necessary to trim? Doesn't it mean we are generating unneeded
whitespace?

https://codereview.chromium.org/27064004/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:862: for (int i = 0; i < _typeArgs.length;
i++) {
Maybe split level=0 and level>0 in two local methods.

https://codereview.chromium.org/27064004/diff/6001/tests/lib/mirrors/class_mi...
File tests/lib/mirrors/class_mirror_type_variables_test.dart (right):

https://codereview.chromium.org/27064004/diff/6001/tests/lib/mirrors/class_mi...
tests/lib/mirrors/class_mirror_type_variables_test.dart:12: class C<Z extends
B<Z>> {}
Add a test for, say, C<Z extends B<C<Z>>>.

[zarah, 2013-10-22 09:03:24]

https://codereview.chromium.org/27064004/diff/6001/sdk/lib/_internal/lib/js_m...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/27064004/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:846:
result.add(reflectClassByMangledName(typeArg.trim()));
On 2013/10/22 06:54:16, Johnni Winther wrote:
> Is it really necessary to trim? Doesn't it mean we are generating unneeded
> whitespace?

No, unfortunately it comes from user code.

https://codereview.chromium.org/27064004/diff/6001/sdk/lib/_internal/lib/js_m...
sdk/lib/_internal/lib/js_mirrors.dart:862: for (int i = 0; i < _typeArgs.length;
i++) {
On 2013/10/22 06:54:16, Johnni Winther wrote:
> Maybe split level=0 and level>0 in two local methods.

As discussed, for simplicity we leave it like this.

https://codereview.chromium.org/27064004/diff/6001/tests/lib/mirrors/class_mi...
File tests/lib/mirrors/class_mirror_type_variables_test.dart (right):

https://codereview.chromium.org/27064004/diff/6001/tests/lib/mirrors/class_mi...
tests/lib/mirrors/class_mirror_type_variables_test.dart:12: class C<Z extends
B<Z>> {}
On 2013/10/22 06:54:16, Johnni Winther wrote:
> Add a test for, say, C<Z extends B<C<Z>>>.

Done.

[zarah, 2013-10-22 10:20:24]

Committed patchset #3 manually as r28991 (presubmit successful).