Skip to content

Commit 07a5def

Browse files
mkustermannCommit Bot
authored and
Commit Bot
committed
Reland "[vm] Avoid type parameter check of _Future._asyncComplete() when caller knows its safe"
Currently we have to do a runtime `as FutureOr<T>` check for the parameter to `_Future<T>.asyncComplete(FutureOr<T> value)`. Although needed for general cases, for the particular usage in the VM's async/await transformer, we know that the test is going to suceed. This CL adds two VM-specific methods on `_Future` that take `dynamic` and downcast via `unsafeCast<>()` to avoid this rather large runtime type checking overhead. Issue #48226 Change after revert: Replace the assert(value is T || value is Future<T>); with assert((value as FutureOr<T>) == value); The former doesn't allow `null` while the ladder might (depending on nullability of `T`). TEST=ci Change-Id: I2379c625003fea6aa836ac74beb1cd59201b3560 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/231704 Reviewed-by: Alexander Markov <[email protected]> Commit-Queue: Martin Kustermann <[email protected]>
1 parent 250173e commit 07a5def

File tree

4 files changed

+23
-3
lines changed

4 files changed

+23
-3
lines changed

pkg/vm/testcases/transformations/type_flow/transformer/enum_from_lib_used_as_type.dart.expect

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,6 @@ class Class extends core::Object {
2222
synthetic constructor •() → self::Class
2323
: super core::Object::•()
2424
;
25-
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3395,getterSelectorId:3396] method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::Enum e) → core::int
25+
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3397,getterSelectorId:3398] method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::Enum e) → core::int
2626
return [@vm.inferred-type.metadata=!] e.{core::_Enum::index}{core::int};
2727
}

pkg/vm/testcases/transformations/type_flow/transformer/tree_shake_enum_from_lib.dart.expect

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,6 @@ class ConstClass extends core::Object {
5151
synthetic constructor •() → self::ConstClass
5252
: super core::Object::•()
5353
;
54-
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3399,getterSelectorId:3400] method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::ConstEnum e) → core::int
54+
[@vm.procedure-attributes.metadata=methodOrSetterCalledDynamically:false,getterCalledDynamically:false,hasThisUses:false,hasTearOffUses:false,methodOrSetterSelectorId:3401,getterSelectorId:3402] method method([@vm.inferred-type.metadata=dart.core::Null? (value: null)] self::ConstEnum e) → core::int
5555
return [@vm.inferred-type.metadata=!] e.{core::_Enum::index}{core::int};
5656
}

sdk/lib/_internal/vm/lib/async_patch.dart

+1-1
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,7 @@ void _completeOnAsyncReturn(_Future _future, Object? value, bool is_sync) {
257257
// allow then and error handlers to be attached.
258258
// async_jump_var=0 is prior to first await, =1 is first await.
259259
if (!is_sync || value is Future) {
260-
_future._asyncComplete(value);
260+
_future._asyncCompleteUnchecked(value);
261261
} else {
262262
_future._completeWithValue(value);
263263
}

sdk/lib/async/future_impl.dart

+20
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,26 @@ class _Future<T> implements Future<T> {
598598
_asyncCompleteWithValue(value as dynamic); // Value promoted to T.
599599
}
600600

601+
/// Internal helper function used by the implementation of `async` functions.
602+
///
603+
/// Like [_asyncComplete], but avoids type checks that are guaranteed to
604+
/// succeed by the way the function is called.
605+
/// Should be used judiciously.
606+
void _asyncCompleteUnchecked(/*FutureOr<T>*/ dynamic value) {
607+
assert((value as FutureOr<T>) == value);
608+
final typedValue = unsafeCast<FutureOr<T>>(value);
609+
610+
// Doing just "is Future" is not sufficient.
611+
// If `T` is Object` and `value` is `Future<Object?>.value(null)`,
612+
// then value is a `Future`, but not a `Future<T>`, and going through the
613+
// `_chainFuture` branch would end up assigning `null` to `Object`.
614+
if (typedValue is Future<T>) {
615+
_chainFuture(typedValue);
616+
return;
617+
}
618+
_asyncCompleteWithValue(unsafeCast<T>(typedValue));
619+
}
620+
601621
void _asyncCompleteWithValue(T value) {
602622
_setPendingComplete();
603623
_zone.scheduleMicrotask(() {

0 commit comments

Comments
 (0)