Inconsistent call method tearoff behavior between analyzer and CFE

[leafpetersen]

The following code behaves inconsistently between CFE and the analyzer:

class A {
  int call(int x) => x;
}

T foo<T>(T Function(T) f) => null;
T bar<T>(T Function(int x) f) =>  null;
T baz<T>(int Function(T x) f) => null;
void main() {
  foo(new A());
  bar(new A());
  baz(new A());
}

The analyzer seems to insert call tearoffs as follows:

void main() {
  foo<dynamic)>(new A());
  bar<dynamic>(new A().call);
  baz<dynamic>(new A());
}

emitting error messages on the call to foo and baz:

  error • The constructor returns type 'A' that isn't of expected type '(dynamic) → dynamic' at /Users/leafp/tmp/ddctest.dart:10:7 • strong_mode_invalid_cast_new_expr
  error • The constructor returns type 'A' that isn't of expected type '(dynamic) → int' at /Users/leafp/tmp/ddctest.dart:12:7 • strong_mode_invalid_cast_new_expr
  warning • A function of type 'A' can't be assigned to a location of type '(dynamic) → dynamic' at /Users/leafp/tmp/ddctest.dart:10:7 • strong_mode_uses_dynamic_as_bottom
  warning • A function of type 'A' can't be assigned to a location of type '(dynamic) → int' at /Users/leafp/tmp/ddctest.dart:12:7 • strong_mode_uses_dynamic_as_bottom

The CFE seems to insert call tearoffs as follows,

void main() {
  foo<dynamic)>(new A().call);
  bar<dynamic>(new A().call);
  baz<dynamic>(new A().call);
}

producing the following error message:

file:///Users/leafp/tmp/ddctest.dart:10:11: Error: A value of type '(dart.core::int) → dart.core::int' can't be assigned to a variable of type '(dynamic) → dynamic'.
Try changing the type of the left hand side, or casting the right hand side to '(dynamic) → dynamic'.
  foo(new A());
          ^ 

If the call to foo is commented out, then a case failure occurs on the call to baz:

Unhandled exception:
type '(int) => int' is not a subtype of type '(dynamic) => int'

There are two issues here:

• The CFE and the analyzer should produce the same choices for the tearoffs.
• I think we should be able to infer these correctly, rather than falling to dynamic.

On the inference front, it seems to me that for foo, we are doing downwards inference in a _ Function(_) context. This forces a tearoff on new A(), and from there the upwards type should be int Function(int) which should then resolve the type argument to foo appropriately.

cc @lrhn @stereotype441 @kmillikin @jmesserly for thoughts on the inference and the tearoff behavior.

[jmesserly]

On the inference front, it seems to me that for foo, we are doing downwards inference in a _ Function(_) context. This forces a tearoff on new A(), and from there the upwards type should be int Function(int) which should then resolve the type argument to foo appropriately.

Agreed. I think it fails in Analyzer because call tearoffs are done in CodeChecker, which happens after Resolver does inference. So we don't have the correct type to work with when we're doing inference. It thinks new A() is type A instead of the implicit tearoff type. I think this should be a pretty easy fix in Analyzer.

@leafpetersen what about this case?

class B<S> {
  S call(S x) => x;
}

T foo<T>(T Function(T) f) => null;
T bar<T>(T Function(int x) f) =>  null;
T baz<T>(int Function(T x) f) => null;
void main() {
  foo(new B());
  bar(new B());
  baz(new B());
}

... do we want to infer the type parameter of B based on the downwards inference context?

[eernstg]

There is no specification of the treatment of casts in dartLangSpec.tex (the "callable object" update only covered static and dynamic invocations), but I think we have had the following general approach on the table during discussions:

If an expression e of a type T which has a method named call is used in a context where a function type F is expected (and by 'function type' I mean a proper subtype of Function which is not a bottom type, or, equivalently, any type which can be written using the syntactic form ... Function(...)), then e is replaced by e.call.

This means that we never support insertion of .call in a downcast from Function or any of its subtypes (even though a Function would be expected to have a call method, we have no idea about which argument list shape it would have), but it means that we would perform the tear-off in all the cases mentioned in the initial message, which is exactly what the CFE is said to do.

Given that generic function instantiation should handle the next step, passing the type argument int to the resulting expression (as in new A().call<int>, ignoring the fact that we don't have this syntax yet), I would expect the example to compile and run successfully, with the following "meaning":

class A {
  int call(int x) => x;
}

T foo<T>(T Function(T) f) => null;
T bar<T>(T Function(int x) f) =>  null;
T baz<T>(int Function(T x) f) => null;

void main() {
  foo<int>(new A().call);
  bar<int>(new A().call);
  baz<int>(new A().call);
}

Double checking: This runs fine with dart2 and has no issues with dartanalyzer --preview-dart-2.

So we might want to discuss downcasts from Function to a function type, but otherwise I think we have already discussed this situation and reached an understanding that should make it work, but we haven't written it anywhere (I'll create a CL for that now).

One more double check, ;-), I removed the actual type arguments on foo, bar, baz, and it turns out that inference does not choose int for all of them (only for bar). I'm not sure whether that's the expected outcome, but it would in any case be a separate topic how to perform that inference step if we agree that the implicit tear-off should take place simply because it's a class with a call which is used where a function type is expected: Even though new A().call might fail, new A() would definitely also fail, so it's not going to help anyone to omit the tear-off (as long as we emit diagnostic messages that the developer can understand).

[eernstg]

Here's a fun one, too:

import 'dart:async';

class A {
  int call(int i) => i;
}

Future<Function> foo() async {
  var result = new Future.value(new A());
  return result;
}

main() async {
  int Function(int) f = await foo();
  print(f(42));
}

This works fine in Dart 1, of course, because the A is actually a working function object. But for Dart 2 we do not have a convenient placement of the call tear-off operation, because the data is passed around via a future. You could say that the data transfer has been "lifted".

In the update to dartLangSpec.tex that I'm currently working on it is not supported to lift a call method extraction like that — if the developer wants that semantics he/she must explicitly await the callable object (var result = await e; return result; rather than var result = e; return result;, when e is a callable object future and a function (future) must be returned). The above example will then remain a compile-time error (saying that we can't assign a Future<A> to a Future<Function>).

[eernstg]

Wrote this CL which contains a proposal for specifying call method extraction (some kinds of actual arguments are not yet covered, but that will be fixed soon).

(Edit Apr 26: The CL now covers all the cases that it should cover, as far as I can see).

[leafpetersen]

Moving this to an analyzer issue. The original example in the bug should be inferred correctly, as it is in the CFE.

[jmesserly]

A related issue in CFE: #33298

[vsmenon]

@bwilkerson @devoncarew @leafpetersen - is there an owner for this? Do we need it for dart2stable?

[jmesserly]

probably. Feel free to assign to me if you need a person.

[MichaelRFairhurst]

I'm the "owner" of all dart 2 stable issues, but I don't assign myself until I've started work.

@jmesserly, if you have cycles for this, that would be super helpful. It looks like you have a good idea of what's required. Basd on your description of the problem, is it just that tearoff insertion has to be moved from CodeChecker to Resolver?

I could definitely take a stab but won't turn down help if its available :)

[MichaelRFairhurst]

I talked to @stereotype441 about this -- its hard to make out all of the information in here and Paul pointed out that it may have changed as well, since the original comments.

However, it seems like in terms of addressing the differences between analyzer and CFE (which is what this ticket is ultimately about), the only real issue is in the error message.

In the CFE, foo & baz call sites produce "(int) -> int is not assignable to (dynamic) -> _". And in the analyzer, they produce "A is not assignable to (dynamic) -> _".

The front-end currently does not do tearoff insertion before inference, and so the analyzer should not (yet) do that either. That's the hard part.

So the easy solution which @jmesserly is recommending, we think, is just to fix the error message in this case, rather than making any inference changes. Notably, this is not quite related to "call tear-off insertion" on the analyzer side, because we don't have any first class support for that (ie, we don't rewrite the AST, etc) but rather we just record the necessary cast from A to (int) -> int. Its possible that what's going wrong is that we don't insert the necessary cast from A to (int) -> int (or (dynamic) -> _) because we see the cast will fail, and that that affects the error message. Its also possible that we have more of a problem of not having adapted the error messages in this case to account for the tear offs because the tear offs only really exist ephemerally or whatever.

If I'm right about this...this seems like a very low priority issue, essentially related to consistency of error messages (or improvement of error messages). I may move this to dart 2.1 stable if there's something under that milestone that seems higher importance.

[dgrove]

@MichaelRFairhurst can you reach a conclusion about moving this to Dart2.1 ? If that is the way to go, then please move this soon.

[srawlins]

The analyzer's and CFE's errors are now perfectly aligned; same text, same errors:

$ xcodebuild/ReleaseX64/dart --enable-experiment=constructor-tearoffs analyze 32864.dart
Analyzing 32864.dart...                1.6s

  error • 32864.dart:9:7 • The argument type 'int Function(int)' can't be assigned to the parameter type 'dynamic Function(dynamic)'. •
          argument_type_not_assignable
  error • 32864.dart:11:7 • The argument type 'int Function(int)' can't be assigned to the parameter type 'int Function(dynamic)'. •
          argument_type_not_assignable

2 issues found.
srawlins-macbookpro2:sdk srawlins$ xcodebuild/ReleaseX64/dart --enable-experiment=constructor-tearoffs 32864.dart
32864.dart:9:11: Error: The argument type 'int Function(int)' can't be assigned to the parameter type 'dynamic Function(dynamic)'.
  foo(new A());
          ^
32864.dart:11:11: Error: The argument type 'int Function(int)' can't be assigned to the parameter type 'int Function(dynamic)'.
  baz(new A());
          ^