Rename members and named constructors.

Rename members and named constructors.

That also leads to renames of named arguments in some contexts like

class C {
  final f;
  C([this.f]);
}

new C(f: 42).

Now we need to change the name of named argument along with the field name.

R=smok@google.com

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

[Roman, 2012-08-29 08:40:08]

lgtm

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:21: 'Map', 'List', 'num',
'int', 'double'
what about bool? There is also LiteralMapEntry, I was just looking at Node
visitor visitLiteral<Something>

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:38: final fixedMemberNames
= new Set<String>();
do NoSuchMethodException and NoSuchMethod will be in this set?

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:61:
fixedMemberNames.add('srcType');
Reading the comment I don't understand why srcType and dstType are here

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/placeholder_collector.dart
(right):

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/placeholder_collector.dart:40:
collector.tryMakeMemberPlaceholder(node.selector);
Just to confirm, this is for calls like "super.foo();" ?

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/placeholder_collector.dart:86: if
(node.receiver is Identifier && node.receiver.asIdentifier().isThis()) {
I don't quite understand when this happens, can you please add a comment with
code sample? (is it when calling named constructor like ": this.named(asdf);"?)

And 80 chars.

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/placeholder_collector.dart:90:
FunctionElement functionElement = element;
Please add a comment explaining what you are doing here. I can understand this
code because we discussed this case:)

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/renamer.dart (right):

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/renamer.dart:14: Set<String>
fixedMemberNames,
I think in this context it's more like 'forbiddenMemberNames' rather than
'fixed'.

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/renamer.dart:19: Generator
topLevelGenerator =
This can be inlined into makeGenerator

https://chromiumcodereview.appspot.com/10887012/diff/1006/tests/compiler/dart...
File tests/compiler/dart2js/unparser_test.dart (right):

https://chromiumcodereview.appspot.com/10887012/diff/1006/tests/compiler/dart...
tests/compiler/dart2js/unparser_test.dart:809: main() {
Please add one complicated test for member renames, something with classes that
have member fields and methods, with named constructors and getters/setters.

[Anton Muhin, 2012-08-29 09:43:45]

ptal

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/backend.dart (right):

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:21: 'Map', 'List', 'num',
'int', 'double'
Thanks a lot for bool, I just forgot about it, adding, good point is this
interface has no methods, so it shouldn't affect anything.

visitLiteralMapEntry I believe is related to "key": value things and thus
doesn't correspond to any real type.

On 2012/08/29 08:40:08, Roman wrote:
> what about bool? There is also LiteralMapEntry, I was just looking at Node
> visitor visitLiteral<Something>

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:38: final fixedMemberNames
= new Set<String>();
On 2012/08/29 08:40:08, Roman wrote:
> do NoSuchMethodException and NoSuchMethod will be in this set?

Yes, they get into this set.  And fortunately enough we have pretty reasonable
coverage for things around no such method.

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/backend.dart:61:
fixedMemberNames.add('srcType');
On 2012/08/29 08:40:08, Roman wrote:
> Reading the comment I don't understand why srcType and dstType are here

Sorry, I meant that those fields are declared in
http://code.google.com/p/dart/source/browse/branches/bleeding_edge/dart/runti...,
but has no correspondence in dart2js version of corelib, which is bad for us.

I tried to reword the comment, your opinion is most appreciated.

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/placeholder_collector.dart
(right):

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/placeholder_collector.dart:40:
collector.tryMakeMemberPlaceholder(node.selector);
On 2012/08/29 08:40:08, Roman wrote:
> Just to confirm, this is for calls like "super.foo();" ?

Exactly

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/placeholder_collector.dart:86: if
(node.receiver is Identifier && node.receiver.asIdentifier().isThis()) {
On 2012/08/29 08:40:08, Roman wrote:
> I don't quite understand when this happens, can you please add a comment with
> code sample? (is it when calling named constructor like ":
this.named(asdf);"?)
> 
> And 80 chars.

Done.

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/placeholder_collector.dart:90:
FunctionElement functionElement = element;
On 2012/08/29 08:40:08, Roman wrote:
> Please add a comment explaining what you are doing here. I can understand this
> code because we discussed this case:)

Done.

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
File lib/compiler/implementation/dart_backend/renamer.dart (right):

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/renamer.dart:14: Set<String>
fixedMemberNames,
I've started with forbidden as well, but actually they are rather fixed: the
idea is you cannot rename something to toString, but you rather have to keep any
member named toString with original name, the rest just follows from the fact
that we don't want to create collisions.

But I agree with you the name is not the best, any suggestions are appreciated.

On 2012/08/29 08:40:09, Roman wrote:
> I think in this context it's more like 'forbiddenMemberNames' rather than
> 'fixed'.

https://chromiumcodereview.appspot.com/10887012/diff/1006/lib/compiler/implem...
lib/compiler/implementation/dart_backend/renamer.dart:19: Generator
topLevelGenerator =
On 2012/08/29 08:40:09, Roman wrote:
> This can be inlined into makeGenerator

But why?  I'd rather use the same generator.

https://chromiumcodereview.appspot.com/10887012/diff/1006/tests/compiler/dart...
File tests/compiler/dart2js/unparser_test.dart (right):

https://chromiumcodereview.appspot.com/10887012/diff/1006/tests/compiler/dart...
tests/compiler/dart2js/unparser_test.dart:809: main() {
I thought about that, but eventually decided against as we have very good
coverage for this stuff by other suites: it hard to rename one member
incorrectly and get reasonably sized test pass.

On 2012/08/29 08:40:09, Roman wrote:
> Please add one complicated test for member renames, something with classes
that
> have member fields and methods, with named constructors and getters/setters.

[Roman, 2012-08-29 10:11:42]

lgtm