Fix for http://code.google.com/p/dart/issues/detail?id=1363: use the simplename for the key, not ...

Fix for http://code.google.com/p/dart/issues/detail?id=1363: use the simplename for the key, not the jsname. Conflicts on the jsname will be dealt during emission.
Committed: https://code.google.com/p/dart/source/detail?r=4066

[kasperl, 2012-02-08 14:00:32]

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/type.dart
File dart/frog/type.dart (right):

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/type.dart#new...
dart/frog/type.dart:1157: var simpleName = '${name}<${Strings.join(names, ',
')}>';
Hmm. Why doesn't the simpleName start with fullname? The type arguments are all
full names now. Maybe this really isn't a simpleName anymore.

Would it make sense to have both a simpleName and a uniqueName (or something
like that)? The simpleName could be passed into the DefinedType constructor. The
uniqueName could be used as the key into the _concreteTypes.

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/type.dart#new...
dart/frog/type.dart:1161: ret = new DefinedType(simpleName, library, definition,
isClass);
Is the name in a DefinedType used for error reporting? Does this change make the
errors harder to read by adding noise to the type argument names?

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/world.dart
File dart/frog/world.dart (right):

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/world.dart#ne...
dart/frog/world.dart:281: final msg = 'conflicting JS name "$name" of same '
Only compute the msg in the case named.isNative.

[ngeoffray, 2012-02-08 16:34:01]

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/type.dart
File dart/frog/type.dart (right):

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/type.dart#new...
dart/frog/type.dart:1157: var simpleName = '${name}<${Strings.join(names, ',
')}>';
On 2012/02/08 14:00:32, kasperl wrote:
> Hmm. Why doesn't the simpleName start with fullname? The type arguments are
all
> full names now. Maybe this really isn't a simpleName anymore.
> 
> Would it make sense to have both a simpleName and a uniqueName (or something
> like that)? The simpleName could be passed into the DefinedType constructor.
The
> uniqueName could be used as the key into the _concreteTypes.

After trying what you suggest, I realized that the 'simpleName' is also used for
uniqueness in other things (for example in the _members map in world.dart).
Therefore, the simpleName (that becomes the name field for the DefinedType) must
be unique.

I believe the reason why it does not have to start with a full name is because
the maps are per-library.

So would you like an extra field 'simpleName' in a type, that will be used to
report errors, or should we just go with one, unique, name?

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/type.dart#new...
dart/frog/type.dart:1161: ret = new DefinedType(simpleName, library, definition,
isClass);
On 2012/02/08 14:00:32, kasperl wrote:
> Is the name in a DefinedType used for error reporting? Does this change make
the
> errors harder to read by adding noise to the type argument names?

It does add noise, but since the 'name' is also used for keys in other maps than
concreteType, we need to put the noise.

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/world.dart
File dart/frog/world.dart (right):

https://chromiumcodereview.appspot.com/9361030/diff/1/dart/frog/world.dart#ne...
dart/frog/world.dart:281: final msg = 'conflicting JS name "$name" of same '
On 2012/02/08 14:00:32, kasperl wrote:
> Only compute the msg in the case named.isNative.

Done.

[kasperl, 2012-02-09 07:10:37]

LGTM. Thanks for trying to simplify it!