Avoid type checks in async completion implementation

[mkustermann]

For an async method like this:

Future<String> foo() async {
  return 'hello world';
}

we generate currently this kernel:

  static method foo() → Future<String>  {
    final _Future<String> :async_future = new _Future::•<String>();
    ...
    bool* :is_sync = false;
    FutureOr<String>? :return_value;
    ...
    function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding 
      ...
       :return_value = "hello world";
      _completeOnAsyncReturn(:async_future, :return_value, :is_sync);
      return;
    }
    ...
    return :async_future;
  }

The value of the return is fully typed all along:

• the future is a _Future<String>
• the resulting value is a FutureOr<String>

Though we loose the correct type when we call _completeOnAsyncReturn, which looks like this:

@pragma("vm:entry-point", "call")
void _completeOnAsyncReturn(_Future _future, Object? value, bool is_sync) {
  if (!is_sync || value is Future) {
    _future._asyncComplete(value);
  } else {
    _future._completeWithValue(value);
  }
}

Notice, the parameter is typed _Future.

The future implementation then looks like this:

class _Future<T>  {
  ...
  void _asyncComplete(FutureOr<T> value) {                                                                          
    if (value is Future<T>) {
      _chainFuture(value);       
      return;     
    }  
    _asyncCompleteWithValue(value as dynamic); // Value promoted to T.   
  }
  ...
}

This function performs

• a type check for the parameter (call site might've up-casted _Future<X> to _Future<Object> ...)
  => Though actually we know the call site, it was generated by kernel transformer, we know that the check will succeed.
  => This is probably an unnecessary check.
• is Future<T>
  => We already know value is FutureOr<T>.
  => Would it be sufficient to just check is Future (instead of is Future<T>) - makes the check faster/simpler?
• the _asyncCompleteWithValue(value as dynamic) will cause an implicit value as T check.
  => We already know that it is T and should not perform any checking here.

/cc @lrhn

(/cc @sstrickl The FutureOr<T> covariant parameter check falls through to STC which overflows)

[mkustermann]

Since we'll have a hard time telling the VM to omit the parameter type check for _Future<T>._asyncComplete(FutureOr<T> value) when coming from that particular callsite, I think it's easier to introduce a new method on _Future that the VM can invoke where no check is performed.

=> Will try cl/230261

[lrhn]

It's generally not sufficient to check is T or is Future to deduce that a FutureOr<T> is a Future<T>.

The counter example in one direction is a FutureOr<Object> which is actually a Future<dynamic>.
Checking just is Future is insufficient to recognize that it's not actually a Future<Object>.

In the other direction, a FutureOr<Object> which is actually a Future<int> is an Object.

We really need to check against Future<T> to be completely certain that we get the correct (and sound) semantics.
(And this is why I want to disallow returning futures directly from async functions, dart-lang/language#870, because there is no good way to implement it efficiently, and the overhead costs even for code which doesn't need it).

As for adding an _asyncCompleteWithValueUnchecked(Object? value) { T typedValue = unsafeCast<T>(value); ... }, it can definitely work (put it in a VM patch file, it's VM specific).
Just be absolutely effing certain it's only being used correctly, because unsoundness is bad.

[mkustermann]

And this is why I want to disallow returning futures directly from async functions, dart-lang/language#870,

Would love to see that land!

there is no good way to implement it efficiently, and the overhead costs even for code which doesn't need it

I have thought of a workaround to make it efficient for the majority of cases: Namely rely on the static types: Recognize that all returns in a function return types that cannot possibly be a Future. For such functions we can then generate code that avoids the is Future<T> check.
-> See cl/230662