Inconsistent runtime errors for implicit method invocations.

[Markzipan]

The following test snippet passes (throws) in DDC/analyzer but doesn't emit an error in the VM:

import "expect.dart";
class C {
  void Function() call = () {};
}

main() {
  C c = new C();
  dynamic d = c;

  var lambda_okay = () => d.call(); 
  var lambda_error = () => d();
  lambda_okay();
  Expect.throws(lambda_error);
}

Ditto for when call is a getter. From what I could find, CFE doesn't provide the backends a way to disambiguate the two cases.

[eernstg]

[Edit] April 5th 2019, I got two things wrong below:

Expect.throws takes a function and will call it, so Expect.throws(lambda_error) is the meaningful thing to do (my reproduction used lambda_error() to avoid the import).

Next, invocation of a value of type dynamic checks for a method and C.call is a getter. As i mentioned ;-), invocation does not happen when call is a getter.

[Original text]

Did you mean Expect.throws(lambda_error())? Evaluation of a local variable will never throw.

Other than that, it is specified that a dynamic invocation of an object whose dynamic type is not a subtype of Function (that is, it's dynamic type is not a function type, because nobody else can implement Function today, and that again means that it is not a function object) must proceed as a (dynamic) invocation of the method call (it can't be a getter).

So d() should amount to d.call() when the value of d is an instance of C, and lamda_error() should just do that.

The behavior that I can see would not be a bug in the front end, but if DDC throws, we might have a DDC bug.

[Markzipan]

The test in question is language_2/call_method_must_not_be_field_test.dart. It has this comment:

  // The presence of a field named `call` does not permit the class `C` to be
  // implicitly called.
  // Nor does it permit an implicit tear-off of `call`.
  // Nor does it permit a dynamic invocation of `call`.
  // However, all these things are possible if `call` is mentioned explicitly.

The test asserts that Expect.throws(() => d()); pass. Does that imply the test is also wrong?

[vsmenon]

Mark is looking at two tests failing DDK but apparently passing DDC and Dart2JS:

language_2/call_method_must_not_be_field_test/06 
language_2/call_method_must_not_be_getter_test/06

@eernstg @jmesserly - can you sanity check that the test is valid?

[jmesserly]

The test looks right to me.

The test asserts that Expect.throws(() => d()); pass. Does that imply the test is also wrong?

Yeah, the comment is unclear, sorry about that. What this comment is trying to say:

The presence of a field named call does not permit the class C to be
implicitly called.
Nor does it permit an implicit tear-off of call.
Nor does it permit a dynamic invocation of call.

Is that a field named call does not work the same as a call method. It does not support the things a call method would support, such as var c = C(); c(); and dynamic d = c; d(). That whole comment is about the first 3 examples, it's discussing the implicit behavior that "call" methods have, and that "call" fields do not have any special behavior.

In all cases, dynamic dispatch and compile time give the same answer. Either it's not allowed (the first group of tests), or it is allowed (the second group). That's why c.call() and d.call() should behave the same.

Another way of saying this: if you rename "call" to "foo" in that test, all of the tests should behave the same. A field with the name "call" is not special. If that helps. I'll try to find the spec.

[jmesserly]

It's in section 16.17.5, "Function Expression Invocation" which has the rules for "call" methods (there is no corresponding rule for "call" fields/getters that happen to return a function).

[eernstg]

@Markzipan wrote:

The test asserts that Expect.throws(() => d()); pass.
Does that imply the test is also wrong?

The test is correct here. I mistakenly thought that call was a method, but call is a getter and d() should throw.

@jmesserly wrote:

A field with the name "call" is not special.

Agreed. Sorry about misreading call as a method and causing the confusion.

[leafpetersen]

Ok, so the question at hand is code that looks like:

  dynamic d = ...
  d();

If d() is treated equivalently to d.call(), then this is valid code regardless of whether d is a function, a class with a call method, or a class with a call getter or field.

If d() is treated as a use of a more primitive notion of calling, then this code is only valid if d is a function or a class with a call method.  It is specifically not valid if d is an instance of a class with a call getter or field.

I believe that we have currently specified the second behavior. 

DDC implements the second behavior, and the front end based tools currently implement the former behavior.

We have a very short window (basically until DDC goes away in favor of DDK) to make this change in a mostly non-breaking way, if we wish to fix it to work as specified (and as implemented in DDC). Alternatively, we could just specify this to work as implemented in the CFE based tools.

For the language team, how much do we care about this distinction?

For the FE/VM/DDC/dart2js folks, how hard would this be to fix?  I'm guessing that the current dart2js behavior is better for the dart2js calling conventions?

cc @kallentu @sigmundch @rakudrama @mraleph @mkustermann @a-siva @johnniwinther @vsmenon

[leafpetersen]

cc @eernstg @munificent @lrhn

[munificent]

If d() is treated equivalently to d.call(), then this is valid code regardless of whether d is a function, a class with a call method, or a class with a call getter or field.

What if it's a class with a call getter that returns a class with a call getter? Consider:

int depth = 0;
class C {
  void Function() get call {
    if (depth > 100) return () {};
    print(depth++);
    return this;
  }
}

main() {
  dynamic d = C();
  d();
}

Would this program be expected to count to 100?

[leafpetersen]

Would this program be expected to count to 100?

With minor edits for typing, that is, in fact, what it currently does on all platforms except DDC.

[eernstg]

@leafpetersen wrote:

I believe that we have currently specified the second behavior.

(which was: with dynamic d, d() can amount to an invocation of a call method, not to an evaluation of a getter followed by an invocation of the returned object). Agreed, cf. this text in the specification:

Otherwise o is not a function object. If o has a method named call
the following ordinary method invocation is evaluated, and its result is
then the result of evaluating i: ... [otherwise noSuchMethod]

Regarding:

a more primitive notion of calling

When I was working on this particular text I made sure to maintain the property that there can be a more primitive notion of function invocation than that of invoking an instance method on an object. This makes it possible to use a faster protocol for invocation: An instance method invocation will in general need a dispatch step (a cheap and well-known one would be a vtable lookup), but an invocation of a function object could rely on having the jump target stored directly in the object. We also made sure that no value whose static type is a function type can be an instance of a user-written class, so every statically checked function invocation could use such a fast invocation mechanism.

I think it is useful to allow for such a primitive notion of calling.

However, I don't think the ability to look up a getter during a dynamic invocation and proceed recursively on the returned result will affect the ability to have these primitive invocations.

We basically omitted getters because the existing specification insisted on finding a method, and it seemed convoluted and not-so-useful to generalize the semantics of function invocation to look for a getter and repeat as well.

[johnniwinther]

The work needed to either support or disallow the invocation on the getter is in the backends. From a static perspective there is no information available about the potential target so CFE cannot handle the case.

[sigmundch]

My vote is to update the spec: it's the path of least resistance (it already works almost everywhere) and it moves the language to be more consistent state (making d() and d.call() equivalent).

Otherwise, the main challenge in dart2js (and I expect the same is true in other backends), is to track the distinction of d() and d.call(). It's doable, but I'm not sure how we can do it without some code-size cost. Today both calls are generated as d.call$0. To distinguish them we'd need to either introduce a new selector (.implicitCall$0?), which would add an extra function definition for every call$0 we have, which can be a lot. Or we'd need to pass some extra argument that we optionally use in the body of the call method (this can potentially be less expensive, but we have yet to estimate how much it would cost us). Given that only DDC has the other semantics, I honestly feel it is not worth changing all the CFE-based tools.

[mraleph]

I vote to change the spec like @sigmundch

I don't think changing VM implementation would face any engineering challenges, but I don't think we should spend time on this given that all non-DDC implementations are aligned and there are no strong reasons for specified behaviour.

[lrhn]

We deliberately made o() and o.call() not equivalent because of the complication introduced by arbitrary getter recursion. If C is a class with a call getter, then C x = ...; x(); is invalid code rejected at compile-time. We want dynamic invocations to have the same behavior as typed invocations, only with run-time errors instead of compile-time errors, so the odd one out here is the dynamic invocation behavior which looks at call getters. Sure, it's already implemented, but it also means that an invocation can have arbitrary complexity, and it differs from typed semantics.

Another, potentially more breaking, approach is to entirely disallow dynamic invocation of non-functions.
Then dynamic x; x(); would only invoke functions, not class objects with call methods.
Invoking Function instances won't change, that type is already only accepting function values.
Even if your class has a call getter method, it won't be called when dynamically called as o(), you have to cast it to Function first (which should also tear off the call method, but not touch a call getter).

It would still require backends to change behavior, but towards a simpler dynamic invocation.

[leafpetersen]

If C is a class with a call getter, then C x = ...; x(); is invalid code rejected at compile-time.

I suppose one option is just to allow this.

Another, potentially more breaking, approach is to entirely disallow dynamic invocation of non-functions.

Is there really any user value in this?

[eernstg]

I created the PR dart-lang/language#594 in order to clarify the changes needed in the specification, in order to allow for a call getter to be used as well as a call method.

So, @lrhn, do you wish to maintain that we shouldn't do this?

[lrhn]

Is that enough? I did not see anything regarding implicit conversion from callable object to Function, likely because it's part of inference, which is also not specified.

We currently say that Function f = o; implicitly tears off call from o if it has a call method, but not a getter, and we want o() to call if o has a call method and no getter.
Both are designed as if they insert a .call tear-off/invocation implicitly.

That is a single .call insertion which is guaranteed to actually provide a function in one step, because a is method always a function. Not so with getters.

Example:

abstract class C<T> {
  T get call;
}
...
C x = ...;
Function f = x;

If call was a method, we'd do a tear-off of the method here.
If call is a getter ... do we get it, and then try to convert that to a Function? That's a recursive definition (we convert to Function by doing something and then converting to Function).

What if x ends up being:

class D implements C<D> {
  D get call => this;
}
C x = D();

Then we are back at an infinite recursion of getting call in order to become a function.

I really, really wanted to get rid of that potential infinite recursion. It's not just in untyped code, as this example shows, and I also don't want dynamic invocations to be more powerful than typed invocations (can call a call getter implicitly dynamically, but not when typed).

It does mean that a dynamic invocation o() is not just a rewrite to o.call(), but I'm fine with that.

So yes, I still want to maintain that we should not treat getters as operator call. (I'd be extatic if we made it an operator declaration, but that's probably a harder sell).

[leafpetersen]

If call is a getter ... do we get it, and then try to convert that to a Function? That's a recursive definition (we convert to Function by doing something and then converting to Function).

If we do this at all, I would propose that no, we simply say that if the result type of the getter is a subtype of function, that's what you get, and otherwise it's a static error.

But I don't think I have any problem with just saying it's a static error. If o is does not have type dynamic and o.call is not a method named call, then o() is an error.

For the dynamic case, we could potentially reduce this (at least temporarily) to a CFE only fix by treating

dynamic d = ...;
d();

as syntactic sugar for

  dynamic d = ...;
  (d.call as Function)();

I think this at least eliminates the infinite recursion.

[eernstg]

In order to resolve this, we can decide that the current specification is what we want (and drop dart-lang/language#594), which means that f() cannot evaluate a getter f.call and call the returned object. Alternatively, we can land dart-lang/language#594.

Implementation Effort: Getter is Error

Here we'd drop dart-lang/language#594.

In order to disallow the getter invocation, we'd need the following implementation effort: Both dart and dart2js will happily compile and run this getter based example (and dartanalyzer of course has no issues with it):

class A {
  Function get call => () { print("A.call"); };
}

main() => (A() as dynamic)();

So dart and dart2js would need to change such that a dynamic error occurs when there is no call method, rather than finding and invoking a getter.

For an invocation which is checked statically we have this example:

class A {
  Function get call => () { print("A.call"); };
}

main() => A()();

This is accepted by the analyzer; it would then have to change such that this example is a compile-time error.

The common front end raises an error in this case (and requires a method), so there are no further changes.

Implementation Effort: Getter is Accepted

Here, we'd land dart-lang/language#594.

The common front end would need to change to not raise a compile-time error for the statically checked case. Code generation might handle this case already, otherwise the generated code must be changed such that a getter invocation will take place.

No changes are needed in the analyzer.

[eernstg]

@leafpetersen, @lrhn, you seem to prefer making the getter invocation an error? @munificent, you also put the focus on the (perhaps excessive) expressive power of repeated getter invocations, wdyt?