Generate better code for '=='.

Generate better code for '==', core.identical, and dart.test.

Avoid falling back to the dart.equal runtime helper where possible based on static types.  Also uses JS `==` or `===` in some cases.

Treat core.identical as a compiler intrinsic, based on type.

Make dart.test assume correct types, use dart.dtest for combined cast and nullability check only when required.

Fixes https://github.com/dart-lang/sdk/issues/29851
Fixes https://github.com/dart-lang/sdk/issues/29772
BUG=
R=jmesserly@google.com

Committed: https://github.com/dart-lang/sdk/commit/6f73694bb7b1f8782ea118cb6a9d9a0732864b8c

[Leaf, 2017-06-23 19:11:23]

Description was changed from

==========
Generate better code for '=='.

Avoid falling back to the dart.equal runtime helper where possible.
Also use JS `==` or `===` in some cases.

BUG=
==========

to

==========
Generate better code for '==', core.identical, and dart.test.

Avoid falling back to the dart.equal runtime helper where possible based on
static types.  Also uses JS `==` or `===` in some cases.

Treat core.identical as a compiler intrinsic, based on type.

Make dart.test assume correct types, use dart.dtest for combined cast and
nullability check only when required.

Fixes https://github.com/dart-lang/sdk/issues/29851
Fixes https://github.com/dart-lang/sdk/issues/29772
BUG=
==========

[Leaf, 2017-06-23 19:24:37]

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

[Leaf, 2017-06-23 19:24:38]

I factored out some of the logic around the JS object model into a helper class.
 Probably more that should be done here.  Take a look, see what you think.  One
question I was undecided about was whether to track Function as separate from
Object.  The JS equality spec doesn't talk about them as different types, but we
treat them a bit differently.

[Jennifer Messerly, 2017-06-26 22:42:59]

Awesome! A lot of these comments are pretty minor factoring stuff.

big ones are:
* I think a file is missing: equality_test.dart
* one of the optimizations may not work for JS interop types, it'd be good to
have a test for that case, and make sure it doesn't call an == method that isn't
there

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:210: objectClass =
_getLibrary(c, 'dart:core').getType('Object'),
we may want to grab the FunctionElement for `identical` based on a few
suggestions below. That can be done with:

final FunctionElement _coreIdentical;

...

    _coreIdentical = _getLibrary(c,
'dart:core').publicNamespace.get('identical'),

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3527: DartType rightType)
{
personally I'd refactor this: remove leftType and rightType, use left.staticType
and right.staticType.

Less parameters simplifies things :)

I'm 99% sure getStaticType is an old obsolete workaround for an analyzer bug.
I'll see if I can get a CL to remove it. worst case, feel free to call
`getStaticType(left)` and `getStaticType(right)` below

Also: do we need to canonicalize the numeric type?

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3532: if
(!typeRep.equalityMayConvert(leftType, rightType)) return true;
this can be simplified too:

return !typeRep.equalityMayConvert(leftType, rightType);

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3537: DartType rightType)
{
leftType and rightType are unused

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3540: if
(!isNullable(left) || !isNullable(right)) return true;
return !isNullable(left) || !isNullable(right);

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3545: if (function is
Identifier) {
based on suggestion above, this can be just:

    return function is Identifier && function.staticElement == _coreIdentical;

(I'd be tempted to name the parameter "node" to save a few chars)

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3555: InvocationExpression
node, JS.Expression fn, List<JS.Expression> args) {
based on suggestions below, the "fn" parameter can be removed

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3556: if (args.length !=
2) return new JS.Call(fn, args);
this can only come up with unsafe flags. I think we could just emit a throw like
we do now in other locations:

  return _callHelper(
    'throw(Error("compile error: calls to `identical` require 2 args")');

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3560: var rightType =
getStaticType(right);
these two lines will go away based on suggestions above

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3562: return js.call('#
=== #', args);
should we also optimize `!identical(left, right)` ?

(discussed further below in the equality case)

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3567: return new
JS.Call(fn, args);
once we have _coreIdentical, it's possible to just:

return new JS.Call(_emitTopLevelName(_coreIdentical), args);

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4182: if
(_tripleEqIsIdentity(left, right, leftType, rightType)) {
it would be really great if this could reuse _emitCoreIdentical.

I think it's possible to factor it so that works. It'll need negation support,
and removing the JS.Expression for "core.identical" (suggested above). But it'd
be nice to share the logic otherwise. You wouldn't necessarily need the separate
functions for _tripleEqIsIdentity/_doubleEqIsIdentity at that point (tho I think
they'd be fine to keep)

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4192: if
(typeRep.objectMethodsOnPrototype(leftType)) {
It looks like you're trying to make sure there's an explicit "==" to call, and
that it's fast to call. That's an impl detail of CodeGenerator and of DDC's core
library/runtime.

Currently == is guaranteed for Dart class types (that aren't JS interop types).
We could put it on JS primitives but that would be slow, so we didn't. We could
add it to functions if we wanted.

// We can call == directly on non-null Dart classes that are
// not JS primitives or interop types.
if (leftType is InterfaceType &&
    !_isJSNative(leftType.element) &&
    typeRep.jsObject(leftType) && 
    !isNullable(left) {
  // ...
}


... the reason I'm putting this suggestion here, is that these details are
specific to `==`, rather than being something about JS Objects or even about all
Dart Object members. (so I think we can remove objectMethodsOnPrototype)

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4204: {
is this block needed?

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4478: expr is NullLiteral
|| getStaticType(expr).isDartCoreNull;
FYI. We may need to also change our nullable analysis to know this.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/js_typerep.dart (right):

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:1: // Copyright (c) 2015, the
Dart project authors.  Please see the AUTHORS file
2017

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:12: abstract class JSType {
this is wonderful!!! thanks for putting this abstraction together :)

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:98: type = (type.element as
TypeParameterElement).bound;
small suggestion:

    (type as TypeParameterType).element.bound

... makes it a little easier to see that the cast is only needed because of our
suboptimal type promotion.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:102: if (type.isBottom) return
JSType.jsNull;
worth a TODO that this needs changing if we get non-nullable types?

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:121: DartType
canonicalizeNumTypes(DartType t) {
perhaps:

    => isNumber(t) ? types.numType : t;

?

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:134: /// Do non-Null instances
of this type definitely have the Dart Object methods
(this may be moot based on another comment below to remove this function)

suggestion for doc style:

/// Returns true iff non-null instances of type [t] have Dart [Object] members
/// on their prototype (including symbolized extension member names).

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:137: bool
objectMethodsOnPrototype(DartType t) {
I think this method won't be needed.

fwiw, almost everything is hooked up to Object.prototype in JS:

    var magic = Symbol("magic");
    Object.prototype[magic] = 42;
    "hello"[magic]; // 42
    0[magic]; // 42
    (x => x)[magic]; // 42

All Dart types including primitives should have the Dart Object members on them
(symbolized if necessary). Looking at our Object helpers, we do seem to have
some special handling for hashCode and toString to be faster on primitives, tho
we appear to assume runtimeType and noSuchMethod and == are available.

Sorry this is all such a mess ... it looks like it's evolved over time :(

I'm gonna hop back over to the caller side of this method and see if I can
suggest something. But I think this method isn't needed here as this file
concerns JS, rather than being concerned with implementation details of DDC's
runtime helpers.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:140: // Functions may not have
the object methods on their prototypes.
FWIW, we could fix this if we wanted to (adding the right extension members to
Function.prototype). probably doesn't matter tho.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/test/c...
File pkg/dev_compiler/test/codegen_expected/equality_test.js (right):

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/test/c...
pkg/dev_compiler/test/codegen_expected/equality_test.js:1: define(['dart_sdk',
'expect'], function(dart_sdk, expect) {
did you mean to also `git add` equality_test.dart?

[Leaf, 2017-06-30 21:55:32]

Ok, I think I got everything, thanks for the detailed comments!  PTAL!

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/code_generator.dart (right):

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:210: objectClass =
_getLibrary(c, 'dart:core').getType('Object'),
On 2017/06/26 22:42:57, Jennifer Messerly wrote:
> we may want to grab the FunctionElement for `identical` based on a few
> suggestions below. That can be done with:
> 
> final FunctionElement _coreIdentical;
> 
> ...
> 
>     _coreIdentical = _getLibrary(c,
> 'dart:core').publicNamespace.get('identical'),

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3527: DartType rightType)
{
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> personally I'd refactor this: remove leftType and rightType, use
left.staticType
> and right.staticType.

Done.

> 
> Less parameters simplifies things :)
> 
> I'm 99% sure getStaticType is an old obsolete workaround for an analyzer bug.
> I'll see if I can get a CL to remove it. worst case, feel free to call
> `getStaticType(left)` and `getStaticType(right)` below
> 
> Also: do we need to canonicalize the numeric type?

No, handled by equalityMayConvert.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3532: if
(!typeRep.equalityMayConvert(leftType, rightType)) return true;
On 2017/06/26 22:42:57, Jennifer Messerly wrote:
> this can be simplified too:
> 
> return !typeRep.equalityMayConvert(leftType, rightType);

Doh.  How embarrassing, I hate code like this.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3537: DartType rightType)
{
On 2017/06/26 22:42:57, Jennifer Messerly wrote:
> leftType and rightType are unused

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3540: if
(!isNullable(left) || !isNullable(right)) return true;
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> return !isNullable(left) || !isNullable(right);

Aargh!  Strong No Hire!

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3545: if (function is
Identifier) {
On 2017/06/26 22:42:57, Jennifer Messerly wrote:
> based on suggestion above, this can be just:
> 
>     return function is Identifier && function.staticElement == _coreIdentical;
> 
> (I'd be tempted to name the parameter "node" to save a few chars)

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3555: InvocationExpression
node, JS.Expression fn, List<JS.Expression> args) {
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> based on suggestions below, the "fn" parameter can be removed

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3556: if (args.length !=
2) return new JS.Call(fn, args);
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> this can only come up with unsafe flags. I think we could just emit a throw
like
> we do now in other locations:
> 
>   return _callHelper(
>     'throw(Error("compile error: calls to `identical` require 2 args")');

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3560: var rightType =
getStaticType(right);
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> these two lines will go away based on suggestions above

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3562: return js.call('#
=== #', args);
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> should we also optimize `!identical(left, right)` ?
> 
> (discussed further below in the equality case)

It's a bit awkward to code up (at first glance, I'd have to put a peephole
optimization in _visitTest to see if the thing being negated is core.identical),
and I'd be very surprised if it made any noticeable performance difference.  V8
should be able to do this optimization itself if it cares.  

I guess maybe generating a core.notIdentical call could be a win, but overall
I'm inclined to leave it for now.  Left a TODO.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:3567: return new
JS.Call(fn, args);
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> once we have _coreIdentical, it's possible to just:
> 
> return new JS.Call(_emitTopLevelName(_coreIdentical), args);

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4182: if
(_tripleEqIsIdentity(left, right, leftType, rightType)) {
On 2017/06/26 22:42:57, Jennifer Messerly wrote:
> it would be really great if this could reuse _emitCoreIdentical.
> 
> I think it's possible to factor it so that works. It'll need negation support,
> and removing the JS.Expression for "core.identical" (suggested above). But
it'd
> be nice to share the logic otherwise. You wouldn't necessarily need the
separate
> functions for _tripleEqIsIdentity/_doubleEqIsIdentity at that point (tho I
think
> they'd be fine to keep)

Done.  It changes the generated code a little bit: we generate core.identical
for equality when we know that equality is identity, but we don't know that we
can use triple or double eq.  But that's probably reasonable.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4192: if
(typeRep.objectMethodsOnPrototype(leftType)) {
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> It looks like you're trying to make sure there's an explicit "==" to call, and
> that it's fast to call. That's an impl detail of CodeGenerator and of DDC's
core
> library/runtime.
> 
> Currently == is guaranteed for Dart class types (that aren't JS interop
types).
> We could put it on JS primitives but that would be slow, so we didn't. We
could
> add it to functions if we wanted.
> 
> // We can call == directly on non-null Dart classes that are
> // not JS primitives or interop types.
> if (leftType is InterfaceType &&
>     !_isJSNative(leftType.element) &&
>     typeRep.jsObject(leftType) && 
>     !isNullable(left) {
>   // ...
> }
> 
> 
> ... the reason I'm putting this suggestion here, is that these details are
> specific to `==`, rather than being something about JS Objects or even about
all
> Dart Object members. (so I think we can remove objectMethodsOnPrototype)

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4204: {
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> is this block needed?

Probably not.  I prefer limiting the scope of local variables with blocks, but
that's probably idiosyncratic in Dart.  Removed.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/code_generator.dart:4478: expr is NullLiteral
|| getStaticType(expr).isDartCoreNull;
On 2017/06/26 22:42:57, Jennifer Messerly wrote:
> FYI. We may need to also change our nullable analysis to know this.

Acknowledged.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
File pkg/dev_compiler/lib/src/compiler/js_typerep.dart (right):

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:1: // Copyright (c) 2015, the
Dart project authors.  Please see the AUTHORS file
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> 2017

I always feel vaguely guilty copy and pasting a copyright notice... :)

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:98: type = (type.element as
TypeParameterElement).bound;
On 2017/06/26 22:42:59, Jennifer Messerly wrote:
> small suggestion:
> 
>     (type as TypeParameterType).element.bound
> 
> ... makes it a little easier to see that the cast is only needed because of
our
> suboptimal type promotion.

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:98: type = (type.element as
TypeParameterElement).bound;
On 2017/06/26 22:42:59, Jennifer Messerly wrote:
> small suggestion:
> 
>     (type as TypeParameterType).element.bound
> 
> ... makes it a little easier to see that the cast is only needed because of
our
> suboptimal type promotion.

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:102: if (type.isBottom) return
JSType.jsNull;
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> worth a TODO that this needs changing if we get non-nullable types?

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:102: if (type.isBottom) return
JSType.jsNull;
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> worth a TODO that this needs changing if we get non-nullable types?

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:121: DartType
canonicalizeNumTypes(DartType t) {
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> perhaps:
> 
>     => isNumber(t) ? types.numType : t;
> 
> ?

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:121: DartType
canonicalizeNumTypes(DartType t) {
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> perhaps:
> 
>     => isNumber(t) ? types.numType : t;
> 
> ?

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:137: bool
objectMethodsOnPrototype(DartType t) {
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> I think this method won't be needed.
> 
> fwiw, almost everything is hooked up to Object.prototype in JS:
> 
>     var magic = Symbol("magic");
>     Object.prototype[magic] = 42;
>     "hello"[magic]; // 42
>     0[magic]; // 42
>     (x => x)[magic]; // 42
> 
> All Dart types including primitives should have the Dart Object members on
them
> (symbolized if necessary). Looking at our Object helpers, we do seem to have
> some special handling for hashCode and toString to be faster on primitives,
tho
> we appear to assume runtimeType and noSuchMethod and == are available.
> 
> Sorry this is all such a mess ... it looks like it's evolved over time :(
> 
> I'm gonna hop back over to the caller side of this method and see if I can
> suggest something. But I think this method isn't needed here as this file
> concerns JS, rather than being concerned with implementation details of DDC's
> runtime helpers.

Done.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/lib/sr...
pkg/dev_compiler/lib/src/compiler/js_typerep.dart:140: // Functions may not have
the object methods on their prototypes.
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> FWIW, we could fix this if we wanted to (adding the right extension members to
> Function.prototype). probably doesn't matter tho.

Acknowledged.

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/test/c...
File pkg/dev_compiler/test/codegen_expected/equality_test.js (right):

https://codereview.chromium.org/2926613003/diff/30001/pkg/dev_compiler/test/c...
pkg/dev_compiler/test/codegen_expected/equality_test.js:1: define(['dart_sdk',
'expect'], function(dart_sdk, expect) {
On 2017/06/26 22:42:58, Jennifer Messerly wrote:
> did you mean to also `git add` equality_test.dart?

I believe this was added in a previous CL, so no diffs.

[Jennifer Messerly, 2017-06-30 22:03:12]

lgtm

[Leaf, 2017-06-30 22:04:39]

Description was changed from

==========
Generate better code for '==', core.identical, and dart.test.

Avoid falling back to the dart.equal runtime helper where possible based on
static types.  Also uses JS `==` or `===` in some cases.

Treat core.identical as a compiler intrinsic, based on type.

Make dart.test assume correct types, use dart.dtest for combined cast and
nullability check only when required.

Fixes https://github.com/dart-lang/sdk/issues/29851
Fixes https://github.com/dart-lang/sdk/issues/29772
BUG=
==========

to

==========
Generate better code for '==', core.identical, and dart.test.

Avoid falling back to the dart.equal runtime helper where possible based on
static types.  Also uses JS `==` or `===` in some cases.

Treat core.identical as a compiler intrinsic, based on type.

Make dart.test assume correct types, use dart.dtest for combined cast and
nullability check only when required.

Fixes https://github.com/dart-lang/sdk/issues/29851
Fixes https://github.com/dart-lang/sdk/issues/29772
BUG=
R=jmesserly@google.com

Committed:
https://github.com/dart-lang/sdk/commit/6f73694bb7b1f8782ea118cb6a9d9a0732864b8c
==========

[Leaf, 2017-06-30 22:04:40]

Committed patchset #4 (id:40001) manually as
6f73694bb7b1f8782ea118cb6a9d9a0732864b8c (presubmit successful).