Wrong async return semantics

[eernstg]

[Edit: This issue is blocked on dart-lang/language#870; see this comment for details.]

Consider the following program:

Future<void> f() async {
  throw 'f';
}

Future<void> g() async {
  try {
    return f();
  } catch (e) {
    print('Caught "$e"');
  }
}

void main() async {
  await g(); // Should print 'Caught "f"'.
}

Execution of this program with dart (from 74be667, and 2.12.0-96.0.dev) gives rise to the following output:

Unhandled exception:
f
#0      f (file:///usr/local/google/home/eernst/lang/dart/scratch/202012/n011.dart:2:3)
dart-lang/sdk#57147      g (file:///usr/local/google/home/eernst/lang/dart/scratch/202012/n011.dart:7:12)
dart-lang/sdk#57148      main (file:///usr/local/google/home/eernst/lang/dart/scratch/202012/n011.dart:14:9)
dart-lang/sdk#57149      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:283:19)
dart-lang/sdk#57150      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:184:12)

However, the semantics of return f() should have been as follows:

When $f$ is an asynchronous non-generator with future value type $T_v$
(\ref{functions}), evaluation proceeds as follows:

($f$ is the function whose body contains the return statement)

The expression $e$ is evaluated to an object $o$.

(In this case, $o$ is an instance of a subtype of Future whose type argument at Future is void.)

If the run-time type of $o$ is a subtype of \code{Future<$T_v$>},

($T_v$ is the future value type of $f$, that is, void. And the subtype relation does hold.)

let \code{v} be a fresh variable bound to $o$ and
evaluate \code{\AWAIT{} v} to an object $r$;
otherwise let $r$ be $o$.
...

The evaluation of said await v will throw, and hence we should proceed to execute the print statement and return from g by completing the body normally (and implicitly completing the returned future with the null object).

The actual behavior indicates that no such evaluation of await v occurs, and the returned future is completed with the thrown exception, so await g() in main throws.

In legacy code the await occurs at the same point, with similar spec language, since commit 5af9844 of Sep 17, 2018.

[johnniwinther]

Given that the expected behavior is depending on the runtime type this is likely a backend issue.

The AST generated from CFE is

library /*isNonNullableByDefault*/;
import self as self;
import "dart:async" as asy;
import "dart:core" as core;

static method f() → asy::Future<void> async {
  throw "f";
}
static method g() → asy::Future<void> async {
  try {
    return self::f();
  }
  on core::Object catch(final core::Object e) {
    core::print("Caught \"${e}\"");
  }
}
static method main() → void async {
  await self::g();
}

and I don't think we should do anything differently.

[eernstg]

I actually get a different output, based on

> FRONT_END_DIR=$HOME/devel/dart/work/sdk/pkg/front_end
> $FRONT_END_DIR/tool/fasta compile --dump-ir n011.dart

The complete output looks like this.

library /*isNonNullableByDefault*/;
import self as self;
import "dart:async" as asy;
import "dart:core" as core;
import "dart:_internal" as _in;

static method f() → asy::Future<void> /* originally async */ {
  final asy::_Future<void> :async_future = new asy::_Future::•<void>();
  core::bool* :is_sync = false;
  FutureOr<void>? :return_value;
  (dynamic) → dynamic :async_op_then;
  (core::Object, core::StackTrace) → dynamic :async_op_error;
  core::int :await_jump_var = 0;
  dynamic :await_ctx_var;
  function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding 
    try {
      #L1:
      {
        throw "f";
      }
      asy::_completeOnAsyncReturn(:async_future, :return_value, :is_sync);
      return;
    }
    on dynamic catch(dynamic exception, core::StackTrace stack_trace) {
      asy::_completeOnAsyncError(:async_future, exception, stack_trace, :is_sync);
    }
  :async_op_then = asy::_asyncThenWrapperHelper(:async_op);
  :async_op_error = asy::_asyncErrorWrapperHelper(:async_op);
  :async_op.call();
  :is_sync = true;
  return :async_future;
}
static method g() → asy::Future<void> /* originally async */ {
  final asy::_Future<void> :async_future = new asy::_Future::•<void>();
  core::bool* :is_sync = false;
  FutureOr<void>? :return_value;
  (dynamic) → dynamic :async_op_then;
  (core::Object, core::StackTrace) → dynamic :async_op_error;
  core::int :await_jump_var = 0;
  dynamic :await_ctx_var;
  function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding 
    try {
      #L2:
      {
        try {
          :return_value = self::f();
          break #L2;
        }
        on core::Object catch(final core::Object e) {
          core::print("Caught \"${e}\"");
        }
      }
      asy::_completeOnAsyncReturn(:async_future, :return_value, :is_sync);
      return;
    }
    on dynamic catch(dynamic exception, core::StackTrace stack_trace) {
      asy::_completeOnAsyncError(:async_future, exception, stack_trace, :is_sync);
    }
  :async_op_then = asy::_asyncThenWrapperHelper(:async_op);
  :async_op_error = asy::_asyncErrorWrapperHelper(:async_op);
  :async_op.call();
  :is_sync = true;
  return :async_future;
}
static method main() → void /* originally async */ {
  final asy::_Future<dynamic> :async_future = new asy::_Future::•<dynamic>();
  core::bool* :is_sync = false;
  FutureOr<dynamic>? :return_value;
  (dynamic) → dynamic :async_op_then;
  (core::Object, core::StackTrace) → dynamic :async_op_error;
  core::int :await_jump_var = 0;
  dynamic :await_ctx_var;
  dynamic :saved_try_context_var0;
  function :async_op([dynamic :result, dynamic :exception, dynamic :stack_trace]) → dynamic yielding 
    try {
      #L3:
      {
        [yield] let dynamic #t1 = asy::_awaitHelper(self::g(), :async_op_then, :async_op_error, :async_op) in null;
        _in::unsafeCast<void>(:result);
      }
      asy::_completeOnAsyncReturn(:async_future, :return_value, :is_sync);
      return;
    }
    on dynamic catch(dynamic exception, core::StackTrace stack_trace) {
      asy::_completeOnAsyncError(:async_future, exception, stack_trace, :is_sync);
    }
  :async_op_then = asy::_asyncThenWrapperHelper(:async_op);
  :async_op_error = asy::_asyncErrorWrapperHelper(:async_op);
  :async_op.call();
  :is_sync = true;
  return :async_future;
}

Here is the crucial part, as I see it:

    try {
      #L2:
      {
        try {
          :return_value = self::f();
          break #L2;
        }
        on core::Object catch(final core::Object e) {
          core::print("Caught \"${e}\"");
        }
      }
      asy::_completeOnAsyncReturn(:async_future, :return_value, :is_sync);
      return;
    }
    on dynamic catch(dynamic exception, core::StackTrace stack_trace) {
      asy::_completeOnAsyncError(:async_future, exception, stack_trace, :is_sync);
    }

The point is that :return_value is assigned the value of self::f() which is the Future<void>, and then _completeOnAsyncReturn will complete :async_future with the exception, rather than throwing at the return statement and catching 4 lines further down.

[johnniwinther]

The difference is due to the VM transformation applied. My example was with --target=none which is without the VM transformation and which matches the AST provided to dart2js and DDC. I checked dart2js also has the wrong semantics, so the problem doesn't lie in the VM transformation.

Would be valid to replace self::f() with await self:f(), and if so, should we do that in the CFE based on static types?

[eernstg]

It's is typically valid to replace self::f() by await self::f(), but the await should not be executed when self::f() returns a future whose type argument does not ensure that the value is a type-correct return value. E.g.:

Future<int> f() {
  return Future<num>.value(1) as dynamic;
}

This should evaluate the returned expression, detect that it is not a subtype of Future<int>, and then encounter a dynamic error (so it should not blindly await the 1 and succeed, because the returned expression yields a future).

So in the case where the returned expression has a type that guarantees that it will be a Future<T> where T is the future value type of the enclosing function, it would be safe to add the wait, but I don't know whether that's useful when it doesn't work in all cases.

I checked dart2js also has the wrong semantics, so the problem
doesn't lie in the VM transformation.

Perhaps it could be a problem in the VM transformation as well as in the back-end processing done by dart2js?

[johnniwinther]

Given that it doesn't work in all cases we need backends to fix this.

I think we should file issue for each backend and make this a meta issue.

[eernstg]

OK!

[johnniwinther]

I haven't checked whether DDC handles this correctly.