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.3k stars 1.59k forks source link

Inconsistent order of evaluation of method lookup in DDC #28963

Open asgerf opened 7 years ago

asgerf commented 7 years ago

In DDC, typed calls evaluate the method lookup in a call before its arguments, whereas dynamic calls evaluate the arguments first.

In other words, for an invocation obj.foo(bar()) it will, roughly speaking, evaluate obj.foo before bar(). This has a few negative consequences:

This example program shows the difference

typedef int IntToInt(int x);

class Obj {
  IntToInt get getter {
    print("getter");
    return (int x) => 4;
  }
}

int argument() {
  print('argument');
  return 3;
}

main() {
  try {
    var obj = new Obj();
    obj.getter(argument());
    obj = null;
    obj.getter(argument());
  } catch (e) {
    print('<exception>');
  }
}

It should print the following (at least in Dart 1.x):

argument
getter
argument
<exception>

But it actually prints:

getter
argument
<exception>

The code generated for main is:

let obj = new foo.Obj();
obj.getter(foo.argument());
obj = null;
obj.getter(foo.argument());

If the type of obj is changed to dynamic, it prints the correct sequence, based on this code:

let obj = new foo.Obj();
dart.dsend(obj, 'getter', foo.argument());
obj = null;
dart.dsend(obj, 'getter', foo.argument());

Background: The root of the issue is that JS evaluates the method lookup before the argument, whereas Dart defers the method lookup until after evaluating the arguments. JavaScript used to have the same evaluation order as Dart, but JS changed since then, and now it is really working against us.

Dart2js has made a lot of sacrifices to work around this issue by aggressively moving subexpressions into local variables before a call with a nullable receiver. It would probably hurt readability a lot to do this in DDC.

Even with non-nullable types, there is the issue of whether a getter is evaluated before or after the arguments.

It would help the kernel team a lot if we can get some clarity on which evaluation order is going to be correct in the future.

@floitschG

floitschG commented 7 years ago

I just looked at dart2js. In many cases it doesn't do the right thing either :( I'm in the process of writing a document and will update this bug with it.

floitschG commented 7 years ago

Document discussing this issue: https://gist.github.com/floitschG/b278ada0316dca96e78c1498d15a2bb9

leafpetersen commented 7 years ago

The proposal to change the evaluation order seemed reasonable to me. @floitschG @munificent @eernstg @lrhn

munificent commented 7 years ago

+1 for changing to strict left-to-right.

As far as noSuchMethod() goes, my personal preference would be to eventually move to something more akin to noSuchProperty(). Dart isn't a method-oriented language like Smalltalk, it's a property-oriented one like JS.

In particular, I think it's weird if the noSuchMethod() behavior of these two lines is different:

(foo.bar)(1);
foo.bar(1);

The language's semantics train users to expect those two lines to mean the same thing, and it's a weird corner of the language with noSuchMethod() where they don't.

lrhn commented 7 years ago

What Bob said.