fix #30030, fix #27327 - fix tearoffs and various Object member bugs

fix #30030, fix #27327 - fix tearoffs and various Object member bugs

this includes various cleanups found along the way:
* various issues with JS annotations, see #30043
* getGenericClass is now optimized when used in a JS builtin with a Type literal
* typed list literals should be slightly faster to create
* memoized getters used by dart:_runtime should be faster
* registerExtension should be a tad faster, and now understands JSArray
* dart.bind "safeName" hack gone in codegenerator
* JSArray<E>.runtimeType returns List<E>
* JSArray<E>.noSuchMethod works
* cleans up code that generates `setExtensionBaseClass` (used only by JSArray)
* _throwUnsafe helper for generating a `throw` in --unsafe compile mode
* fix tearoff of JS interop types (_emitSimpleIdentifier)
* fix TODO about splitting LazyJSType and AnonymousJSType
* fix is/as/cast methods on TypeRep and related classes
* fix codegenerator to not tearoff "hashCode" and "runtimeType" (they are not methods)
* fix NoSuchMethodError.toString()   (update via dart2js sdk impl)

R=vsm@google.com

Committed: https://github.com/dart-lang/sdk/commit/5533a256cbb12c9db30650bf2418fb2a872f6854

[Jennifer Messerly, 2017-06-30 03:14:10]

Description was changed from

==========
fix #30030, fix #27327 - fix tearoffs and various Object member bugs
==========

to

==========
fix #30030, fix #27327 - fix tearoffs and various Object member bugs

this also cleans up a lot of the issues with #30043
various other cleanups:
* getGenericClass is now optimized when used in a JS builtin with a Type literal
   - notably, this should speed up typed list literals
* some optimizations for memoized getters
* dart.bind "safeName" hack gone
* registerExtension should be a tad faster, and now understands JSArray
* JSArray<E>.runtimeType returns List<E>
* JSArray<E>.noSuchMethod works
* cleans up code that generates `setExtensionBaseClass` (used only by JSArray)
* _throwUnsafe helper for generating a `throw` in --unsafe compile mode
* fix tearoff of JS methods in some cases
* fix TODO about splitting LazyJSType and AnonymousJSType
* fix is/as/cast methods on TypeRep and related classes
* fix tearoff code to not kick in for "hashCode" and "runtimeType" (they are not
methods)
* fix NoSuchMethodError.toString()   (copy+paste from dart2js SDK)
==========

[Jennifer Messerly, 2017-06-30 03:17:03]

Description was changed from

==========
fix #30030, fix #27327 - fix tearoffs and various Object member bugs

this also cleans up a lot of the issues with #30043
various other cleanups:
* getGenericClass is now optimized when used in a JS builtin with a Type literal
   - notably, this should speed up typed list literals
* some optimizations for memoized getters
* dart.bind "safeName" hack gone
* registerExtension should be a tad faster, and now understands JSArray
* JSArray<E>.runtimeType returns List<E>
* JSArray<E>.noSuchMethod works
* cleans up code that generates `setExtensionBaseClass` (used only by JSArray)
* _throwUnsafe helper for generating a `throw` in --unsafe compile mode
* fix tearoff of JS methods in some cases
* fix TODO about splitting LazyJSType and AnonymousJSType
* fix is/as/cast methods on TypeRep and related classes
* fix tearoff code to not kick in for "hashCode" and "runtimeType" (they are not
methods)
* fix NoSuchMethodError.toString()   (copy+paste from dart2js SDK)
==========

to

==========
fix #30030, fix #27327 - fix tearoffs and various Object member bugs

this includes various cleanups found along the way:
* various issues with JS annotations, see #30043
* getGenericClass is now optimized when used in a JS builtin with a Type literal
* typed list literals should be slightly faster to create
* memoized getters used by dart:_runtime should be faster
* registerExtension should be a tad faster, and now understands JSArray
* dart.bind "safeName" hack gone in codegenerator
* JSArray<E>.runtimeType returns List<E>
* JSArray<E>.noSuchMethod works
* cleans up code that generates `setExtensionBaseClass` (used only by JSArray)
* _throwUnsafe helper for generating a `throw` in --unsafe compile mode
* fix tearoff of JS interop types (_emitSimpleIdentifier)
* fix TODO about splitting LazyJSType and AnonymousJSType
* fix is/as/cast methods on TypeRep and related classes
* fix codegenerator to not tearoff "hashCode" and "runtimeType" (they are not
methods)
* fix NoSuchMethodError.toString()   (update via dart2js sdk impl)
==========

[Jennifer Messerly, 2017-06-30 03:17:03]

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

[vsm, 2017-06-30 18:08:48]

lgtm

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:385: String
_getJSNameWithoutGlobal(Element e) {
This will still support something like:

@JS('foo.bar.baz')
external get baz;

right?  Looks like we were returning a List<String> only to join it together
later on.  I think you're just skipping that unneeded step.

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:390: : _isJSNative(e);
Maybe this should e.isExternal || _isJSNative(e) ?  Or the former should be
folded into the latter?  I think external can appear non-top level.

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:1667:
'setExtensionBaseClass(#, #);', [className, newBaseClass]));
Ha - I was just trying to figure why JSArray's proto was different from
everything else while looking at cross-frame code.  Thanks for the pointer.  :-)

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3104: (e is
PropertyAccessorElement ? e.variable.name : e.name);
Consider factoring out - this guarded expr seems frequent.

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4270: var elmement =
id.staticElement;
elmement -> element

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:5447: // whether it'll be
implemented via forwarding.
"whether" -> "know whether"

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/tool/input...
File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/classes.dart (right):

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/tool/input...
pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/classes.dart:418: if
(JS('bool', '# === #', jsProto, dartProto)) return;
This was there before, but I'm not sure I understand why/when this line above
kicks in.

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/tool/input...
File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart
(right):

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/tool/input...
pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart:57: //
Another idea here is to have a type that maps to Funtion.prototype,
Funtion -> Function

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/tool/input...
File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/types.dart (right):

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/tool/input...
pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/types.dart:62: bool
is_T(object) => instanceOf(object, this);
nice :-)

[Jennifer Messerly, 2017-07-05 21:17:16]

Thanks! I think I've addressed all of these so landing, but let me know if I
missed anything :)

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:385: String
_getJSNameWithoutGlobal(Element e) {
On 2017/06/30 18:08:47, vsm wrote:
> This will still support something like:
> 
> @JS('foo.bar.baz')
> external get baz;
> 
> right?  Looks like we were returning a List<String> only to join it together
> later on.  I think you're just skipping that unneeded step.

exactly, it just skips the redundant step. I believe there's a test for this too
(under lib/html).

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:390: : _isJSNative(e);
On 2017/06/30 18:08:47, vsm wrote:
> Maybe this should e.isExternal || _isJSNative(e) ?  Or the former should be
> folded into the latter?  I think external can appear non-top level.

That's what I was wondering too. This is exactly the same logic we used before,
I was refactoring it to figure out what as going on with `external` and this was
my conclusion :)

It didn't make sense to me either. After a chat with JacobR I've refactored it
to make more sense. Everything needs `external` except classes, near as I can
tell.

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:1667:
'setExtensionBaseClass(#, #);', [className, newBaseClass]));
On 2017/06/30 18:08:47, vsm wrote:
> Ha - I was just trying to figure why JSArray's proto was different from
> everything else while looking at cross-frame code.  Thanks for the pointer. 
:-)

yup! it's pretty crazy ... hope it's a tad better documented now

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3104: (e is
PropertyAccessorElement ? e.variable.name : e.name);
On 2017/06/30 18:08:47, vsm wrote:
> Consider factoring out - this guarded expr seems frequent.

Done.

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4270: var elmement =
id.staticElement;
On 2017/06/30 18:08:47, vsm wrote:
> elmement -> element

Done.

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/lib/src/co...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:5447: // whether it'll be
implemented via forwarding.
On 2017/06/30 18:08:48, vsm wrote:
> "whether" -> "know whether"

oh funny, we must've missed this in the original CL (this typo was in the
original code). But yeah, good catch, fixed!

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/tool/input...
File pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/classes.dart (right):

https://codereview.chromium.org/2962263002/diff/1/pkg/dev_compiler/tool/input...
pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/classes.dart:418: if
(JS('bool', '# === #', jsProto, dartProto)) return;
On 2017/06/30 18:08:47, vsm wrote:
> This was there before, but I'm not sure I understand why/when this line above
> kicks in.

Yeah, I'm not sure! It might've been a guard against Array.prototype (tho we
still had useless copy properties) or it might've been to stop at JS
Object.prototype. In the new code, neither of those should be a problem so I'll
see if I can clean it up :)

[Jennifer Messerly, 2017-07-05 21:18:05]

Description was changed from

==========
fix #30030, fix #27327 - fix tearoffs and various Object member bugs

this includes various cleanups found along the way:
* various issues with JS annotations, see #30043
* getGenericClass is now optimized when used in a JS builtin with a Type literal
* typed list literals should be slightly faster to create
* memoized getters used by dart:_runtime should be faster
* registerExtension should be a tad faster, and now understands JSArray
* dart.bind "safeName" hack gone in codegenerator
* JSArray<E>.runtimeType returns List<E>
* JSArray<E>.noSuchMethod works
* cleans up code that generates `setExtensionBaseClass` (used only by JSArray)
* _throwUnsafe helper for generating a `throw` in --unsafe compile mode
* fix tearoff of JS interop types (_emitSimpleIdentifier)
* fix TODO about splitting LazyJSType and AnonymousJSType
* fix is/as/cast methods on TypeRep and related classes
* fix codegenerator to not tearoff "hashCode" and "runtimeType" (they are not
methods)
* fix NoSuchMethodError.toString()   (update via dart2js sdk impl)
==========

to

==========
fix #30030, fix #27327 - fix tearoffs and various Object member bugs

this includes various cleanups found along the way:
* various issues with JS annotations, see #30043
* getGenericClass is now optimized when used in a JS builtin with a Type literal
* typed list literals should be slightly faster to create
* memoized getters used by dart:_runtime should be faster
* registerExtension should be a tad faster, and now understands JSArray
* dart.bind "safeName" hack gone in codegenerator
* JSArray<E>.runtimeType returns List<E>
* JSArray<E>.noSuchMethod works
* cleans up code that generates `setExtensionBaseClass` (used only by JSArray)
* _throwUnsafe helper for generating a `throw` in --unsafe compile mode
* fix tearoff of JS interop types (_emitSimpleIdentifier)
* fix TODO about splitting LazyJSType and AnonymousJSType
* fix is/as/cast methods on TypeRep and related classes
* fix codegenerator to not tearoff "hashCode" and "runtimeType" (they are not
methods)
* fix NoSuchMethodError.toString()   (update via dart2js sdk impl)

R=vsm@google.com

Committed:
https://github.com/dart-lang/sdk/commit/5533a256cbb12c9db30650bf2418fb2a872f6854
==========

[Jennifer Messerly, 2017-07-05 21:18:06]

Committed patchset #2 (id:10001) manually as
5533a256cbb12c9db30650bf2418fb2a872f6854 (presubmit successful).