fixes #232, handling of arguments

fixes #232, handling of arguments
our renamer wasn't trusting the JS builtin to use arguments correctly :)
incidentally, this does the final step in making the renamer precise: it tracks all renames per-scope now. In practice this was almost never hit except for "arguments" though.

R=vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/1b3fbf3e972f14ed6482aad4dfaafa2302841a72

[Jennifer Messerly, 2015-06-23 18:40:12]

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

[vsm, 2015-06-23 19:58:28]

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

https://codereview.chromium.org/1200243002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:851: if (name in this) result =
this[name](...arguments);
Shouldn't this be something like:

if (this.__proto__.hasOwnProperty(name)) ....

?

I think we'd handle the following wrong otherwise:

class Base {
  Derived() => print("This shouldn't be called");
}

class Derived extends Base {}

main() => print(new Derived());

[Jennifer Messerly, 2015-06-23 20:08:35]

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

https://codereview.chromium.org/1200243002/diff/1/lib/src/codegen/js_codegen....
lib/src/codegen/js_codegen.dart:851: if (name in this) result =
this[name](...arguments);
On 2015/06/23 19:58:27, vsm wrote:
> Shouldn't this be something like:
> 
> if (this.__proto__.hasOwnProperty(name)) ....
> 
> ?
> 
> I think we'd handle the following wrong otherwise:
> 
> class Base {
>   Derived() => print("This shouldn't be called");
> }
> 
> class Derived extends Base {}
> 
> main() => print(new Derived());

the problem, if it repros, already exists in old code. The solution would be to
make sure initialize method is generated in that case (it might already be. not
sure), and/or rename it (e.g. a dart.initialize symbol). I'd hate to need a
hasOwnProperty check here... that would probably add more overhead to this
already-crazy code path. Fortunately we have a few issues tracking cleanup here.

anyway, I can revert this if you want, just figured it would be nice to use
spread instead of .apply. I definitely don't want to get into the business of
fixing constructors in this CL, though :)

[Jennifer Messerly, 2015-06-23 20:12:05]

the other bug is tracked in https://github.com/dart-lang/dev_compiler/issues/163
and https://github.com/dart-lang/dev_compiler/issues/162

[vsm, 2015-06-23 20:18:24]

lgtm - yeah, old code has the same problem.  thanks for updating the bug.

[Jennifer Messerly, 2015-06-23 20:35:44]

Committed patchset #1 (id:1) manually as
1b3fbf3e972f14ed6482aad4dfaafa2302841a72 (presubmit successful).