Substitute types in generic superclasses and interfaces.

Substitute types in generic superclasses and interfaces.

R=johnniwinther@google.com

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

[karlklose, 2013-10-29 15:14:09]

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1411: var result = const [];
List<ClassMirror> result = const <ClassMirror>[];

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1926: return -1;
You could just throw here.

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1930: var type) {
You could document the possible types like this
  'var /* int|List|... */ type'
or add this
  'assert(type is int || type is List || ...)'.

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1946: TypeVariable typeVariable = JS('',
'init.metadata[#]', type);
Perhaps add a helper to get metadata:
 getMetaData(int index) => JS('', 'init.metadata[#]', index);

https://codereview.chromium.org/47743011/diff/120001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_superclass_test.dart (right):

https://codereview.chromium.org/47743011/diff/120001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_superclass_test.dart:75: ClassMirror superA=
a.superclass;
Space before =.

[Johnni Winther, 2013-10-30 10:27:18]

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1022: if (typeInformation != null) {
Change to assert(typeInformation != null).

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1375: // Use _superclass == this to
represent class with no superclass (Object).
Long line.

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
File sdk/lib/_internal/lib/js_rti.dart (right):

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_rti.dart:137: String getRuntimeTypeAsString(var
runtimeType, {String onTypeVariable(int)}) {
'int' -> 'int i', otherwise it is just a variable named 'int'.

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_rti.dart:153: String runtimeTypeToString(var type ,
{String onTypeVariable(int)}) {
Ditto.

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_rti.dart:178: String joinArguments(var types, int
startIndex, {String onTypeVariable(int)}) {
Ditto.

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_bounded_by_type_parameter_test.dart (left):

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_bounded_by_type_parameter_test.dart:61:
typeArguments(superOfNumAndInt, [reflectClass(num), reflectClass(int)]);
Why this change?

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_bounded_by_type_parameter_test.dart (right):

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_bounded_by_type_parameter_test.dart:23: int i =
'string';
Change to

var s = 'string';
int i = s;

to avoid static warnings on the assignment.

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_bounded_test.dart (right):

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_bounded_test.dart:21: int i = 'string';
Ditto.

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_superclass_test.dart (right):

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_superclass_test.dart:29: ClassMirror superA=
a.superclass;
Add space before =. Similar below.

[zarah, 2013-10-30 11:51:39]

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1411: var result = const [];
On 2013/10/29 15:14:09, karlklose wrote:
> List<ClassMirror> result = const <ClassMirror>[];

Done.

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1926: return -1;
On 2013/10/29 15:14:09, karlklose wrote:
> You could just throw here.

Done.

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1930: var type) {
On 2013/10/29 15:14:09, karlklose wrote:
> You could document the possible types like this
>   'var /* int|List|... */ type'
> or add this
>   'assert(type is int || type is List || ...)'.

Done.

https://codereview.chromium.org/47743011/diff/120001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1946: TypeVariable typeVariable = JS('',
'init.metadata[#]', type);
On 2013/10/29 15:14:09, karlklose wrote:
> Perhaps add a helper to get metadata:
>  getMetaData(int index) => JS('', 'init.metadata[#]', index);

Done.

https://codereview.chromium.org/47743011/diff/120001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_superclass_test.dart (right):

https://codereview.chromium.org/47743011/diff/120001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_superclass_test.dart:75: ClassMirror superA=
a.superclass;
On 2013/10/29 15:14:09, karlklose wrote:
> Space before =.

Done.

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1022: if (typeInformation != null) {
On 2013/10/30 10:27:18, Johnni Winther wrote:
> Change to assert(typeInformation != null).

Done.

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_mirrors.dart:1375: // Use _superclass == this to
represent class with no superclass (Object).
On 2013/10/30 10:27:18, Johnni Winther wrote:
> Long line.

Done.

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
File sdk/lib/_internal/lib/js_rti.dart (right):

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_rti.dart:137: String getRuntimeTypeAsString(var
runtimeType, {String onTypeVariable(int)}) {
On 2013/10/30 10:27:18, Johnni Winther wrote:
> 'int' -> 'int i', otherwise it is just a variable named 'int'.

Done.

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_rti.dart:153: String runtimeTypeToString(var type ,
{String onTypeVariable(int)}) {
On 2013/10/30 10:27:18, Johnni Winther wrote:
> Ditto.

Done.

https://codereview.chromium.org/47743011/diff/210001/sdk/lib/_internal/lib/js...
sdk/lib/_internal/lib/js_rti.dart:178: String joinArguments(var types, int
startIndex, {String onTypeVariable(int)}) {
On 2013/10/30 10:27:18, Johnni Winther wrote:
> Ditto.

Done.

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_bounded_by_type_parameter_test.dart (left):

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_bounded_by_type_parameter_test.dart:61:
typeArguments(superOfNumAndInt, [reflectClass(num), reflectClass(int)]);
On 2013/10/30 10:27:18, Johnni Winther wrote:
> Why this change?

Removed, as discussed.

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_bounded_by_type_parameter_test.dart (right):

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_bounded_by_type_parameter_test.dart:23: int i =
'string';
On 2013/10/30 10:27:18, Johnni Winther wrote:
> Change to
> 
> var s = 'string';
> int i = s;
> 
> to avoid static warnings on the assignment.

Done.

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_bounded_test.dart (right):

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_bounded_test.dart:21: int i = 'string';
On 2013/10/30 10:27:18, Johnni Winther wrote:
> Ditto.

Done.

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
File tests/lib/mirrors/generic_superclass_test.dart (right):

https://codereview.chromium.org/47743011/diff/210001/tests/lib/mirrors/generi...
tests/lib/mirrors/generic_superclass_test.dart:29: ClassMirror superA=
a.superclass;
On 2013/10/30 10:27:18, Johnni Winther wrote:
> Add space before =. Similar below.

Done.

[Johnni Winther, 2013-10-30 11:53:43]

lgtm

https://codereview.chromium.org/47743011/diff/90002/sdk/lib/_internal/lib/js_...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/47743011/diff/90002/sdk/lib/_internal/lib/js_...
sdk/lib/_internal/lib/js_mirrors.dart:1952: int variableIndex =
findTypeVariableIndex(typeVariables, typeVariable.name);
Long line.

[zarah, 2013-10-30 14:01:43]

Committed patchset #6 manually as r29545 (presubmit successful).

[zarah, 2013-10-30 15:01:35]

Thanks!

https://codereview.chromium.org/47743011/diff/90002/sdk/lib/_internal/lib/js_...
File sdk/lib/_internal/lib/js_mirrors.dart (right):

https://codereview.chromium.org/47743011/diff/90002/sdk/lib/_internal/lib/js_...
sdk/lib/_internal/lib/js_mirrors.dart:1952: int variableIndex =
findTypeVariableIndex(typeVariables, typeVariable.name);
On 2013/10/30 11:53:43, Johnni Winther wrote:
> Long line.

Done.