dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.07k stars 1.56k forks source link

Make fasta lower `== null` to `identical()` in various cases #31799

Open mkustermann opened 6 years ago

mkustermann commented 6 years ago

Currently fasta lowers comparisons with null, e.g. in

main() {
  A f;
  print(f == null ? 'null' : 'non-null');
  f ??= new A();
}
class A {}

to

  class A extends core::Object {
    default constructor •() → void  : super core::Object::•();
  }
  static method main() → dynamic {
    test::A f;
    core::print(f.{core::Object::==}(null) ?{core::String} "null" : "non-null");
    f.{core::Object::==}(null) ?{test::A} f = new test::A::•() : null;
  }

It should perform this lowering by using identical(<foo>, null) instead!

One advantage of this is that it is not valid to use the == operator on non-primitive constants, but it is valid to use identical(<constant>, null).

mkustermann commented 6 years ago

/cc @mraleph

lrhn commented 6 years ago

The text f.{core::Object::==}(null) suggests that you actually call the == method of f with a null argument. Is this the case? If so, it is a bug. The == operator syntax is specified to never call the == method on the receiver if either operand is the null value. It must handle that locally (by checking whether the other operand is also null).

mraleph commented 6 years ago

@lrhn checking locally is too much of a size overhead. Instead this is handled by backends, e.g. VM injects check for null into prologue of every operator == definition (which is not observable).

lrhn commented 6 years ago

That makes sense. It does require every backend to know that calls to operator== are different from other method calls (but "every backend" isn't that many, so it's just unrelated people like me who gets confused). Thank you for the explanation.

rakudrama commented 6 years ago

@lrhn dart2js also injects a null check in every operator == definition in order to keep call sites small. dart2js recognizes x == null and lowers it to an instruction form of identical(x, null) in the optimizer.

If the size overhead is a concern for this frequent check, Kernel could add a special 'is-null' node. I would use such an operation only for checks inserted by lowering, and not user code, otherwise we lose the source location of the arguments to operator==, making it difficult to generate accurate sourcemaps.

mkustermann commented 6 years ago

Is there any updates on this?

@peter-ahe-google

peter-ahe-google commented 6 years ago

No. This isn't really on our radar.