Skip to content

Commit c81711b

Browse files
fishythefishCommit Queue
authored and
Commit Queue
committed
[dart2js] Add runtime type check for await.
See #49396 for details. Fixes: #50601 Change-Id: Ie89130cffe642b3e4935d7d02fe2e34f7fc8b12e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/316320 Commit-Queue: Mayank Patke <[email protected]> Reviewed-by: Stephen Adams <[email protected]>
1 parent 585c34b commit c81711b

File tree

9 files changed

+126
-4
lines changed

9 files changed

+126
-4
lines changed

pkg/compiler/lib/src/common/elements.dart

+4
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ abstract class CommonElements {
8888
/// The `Future` class defined in 'async';.
8989
late final ClassEntity futureClass = _findClass(asyncLibrary, 'Future');
9090

91+
/// The `Future.value` constructor.
92+
late final ConstructorEntity? futureValueConstructor =
93+
_env.lookupConstructor(futureClass, 'value');
94+
9195
/// The `Stream` class defined in 'async';
9296
late final ClassEntity streamClass = _findClass(asyncLibrary, 'Stream');
9397

pkg/compiler/lib/src/ir/impact.dart

+2
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ abstract class ImpactRegistry {
5858

5959
void registerSyncStar(ir.DartType elementType);
6060

61+
void registerAwaitCheck();
62+
6163
void registerAsync(ir.DartType elementType);
6264

6365
void registerAsyncStar(ir.DartType elementType);

pkg/compiler/lib/src/ir/impact_data.dart

+30
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import 'package:kernel/ast.dart' as ir;
66
import 'package:kernel/class_hierarchy.dart' as ir;
7+
import 'package:kernel/core_types.dart' as ir;
78
import 'package:kernel/type_environment.dart' as ir;
89

910
import '../common.dart';
@@ -50,6 +51,8 @@ class ImpactBuilder extends StaticTypeVisitor implements ImpactRegistry {
5051
: super(
5152
staticTypeContext.typeEnvironment, classHierarchy, staticTypeCache);
5253

54+
ir.CoreTypes get _coreTypes => typeEnvironment.coreTypes;
55+
5356
@override
5457
VariableScopeModel get variableScopeModel => _variableScopeModel!;
5558

@@ -607,6 +610,24 @@ class ImpactBuilder extends StaticTypeVisitor implements ImpactRegistry {
607610
.visitConstant(node.constant);
608611
}
609612

613+
@override
614+
void handleAwaitExpression(ir.AwaitExpression node) {
615+
final runtimeCheckType = node.runtimeCheckType;
616+
if (runtimeCheckType != null) {
617+
// Register the impacts which would have been registered if this were
618+
// implemented directly as Dart code. Some of these may be redundant with
619+
// the impacts we already register for `async` code, but we include them
620+
// for completeness.
621+
registerAwaitCheck();
622+
registerIsCheck(runtimeCheckType);
623+
final typeArgument =
624+
(runtimeCheckType as ir.InterfaceType).typeArguments.single;
625+
registerNew(_coreTypes.futureValueFactory, runtimeCheckType, 1, const [],
626+
[typeArgument], getDeferredImport(node),
627+
isConst: false);
628+
}
629+
}
630+
610631
void _registerFeature(_Feature feature) {
611632
(_data._features ??= EnumSet<_Feature>()).add(feature);
612633
}
@@ -881,6 +902,11 @@ class ImpactBuilder extends StaticTypeVisitor implements ImpactRegistry {
881902
_registerTypeUse(elementType, _TypeUseKind.asyncMarker);
882903
}
883904

905+
@override
906+
void registerAwaitCheck() {
907+
_registerFeature(_Feature.awaitCheck);
908+
}
909+
884910
@override
885911
void registerSyncStar(ir.DartType elementType) {
886912
_registerTypeUse(elementType, _TypeUseKind.syncStarMarker);
@@ -1395,6 +1421,9 @@ class ImpactData {
13951421
case _Feature.intLiteral:
13961422
registry.registerIntLiteral();
13971423
break;
1424+
case _Feature.awaitCheck:
1425+
registry.registerAwaitCheck();
1426+
break;
13981427
}
13991428
}
14001429
}
@@ -1903,6 +1932,7 @@ enum _Feature {
19031932
intLiteral,
19041933
symbolLiteral,
19051934
doubleLiteral,
1935+
awaitCheck,
19061936
}
19071937

19081938
class _TypeUse {

pkg/compiler/lib/src/ir/static_type.dart

+8
Original file line numberDiff line numberDiff line change
@@ -1838,6 +1838,14 @@ abstract class StaticTypeVisitor extends StaticTypeBase {
18381838
handleConstantExpression(node);
18391839
return super.visitConstantExpression(node);
18401840
}
1841+
1842+
void handleAwaitExpression(ir.AwaitExpression node) {}
1843+
1844+
@override
1845+
ir.DartType visitAwaitExpression(ir.AwaitExpression node) {
1846+
handleAwaitExpression(node);
1847+
return super.visitAwaitExpression(node);
1848+
}
18411849
}
18421850

18431851
class ArgumentTypes {

pkg/compiler/lib/src/js_backend/backend_impact.dart

+6
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@ class BackendImpacts {
110110
],
111111
);
112112

113+
late final BackendImpact awaitExpression = BackendImpact(
114+
staticUses: [
115+
_commonElements.futureValueConstructor!,
116+
],
117+
);
118+
113119
late final BackendImpact asyncBody = BackendImpact(
114120
staticUses: [
115121
_commonElements.asyncHelperAwait,

pkg/compiler/lib/src/js_backend/backend_usage.dart

+1
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,7 @@ class BackendUsageBuilderImpl implements BackendUsageBuilder {
171171
bool _isValidEntity(Entity element) {
172172
if (element is ConstructorEntity &&
173173
(element == _commonElements.streamIteratorConstructor ||
174+
element == _commonElements.futureValueConstructor ||
174175
_commonElements.isSymbolConstructor(element))) {
175176
// TODO(johnniwinther): These are valid but we could be more precise.
176177
return true;

pkg/compiler/lib/src/kernel/kernel_impact.dart

+5
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,11 @@ class KernelImpactConverter implements ImpactRegistry {
201201
<DartType>[elementMap.getDartType(elementType)]));
202202
}
203203

204+
@override
205+
void registerAwaitCheck() {
206+
registerBackendImpact(_impacts.awaitExpression);
207+
}
208+
204209
@override
205210
void registerAsync(ir.DartType elementType) {
206211
registerBackendImpact(_impacts.asyncBody);

pkg/compiler/lib/src/ssa/builder.dart

+57-4
Original file line numberDiff line numberDiff line change
@@ -6060,11 +6060,64 @@ class KernelSsaGraphBuilder extends ir.Visitor<void> with ir.VisitorVoidMixin {
60606060

60616061
@override
60626062
void visitAwaitExpression(ir.AwaitExpression node) {
6063-
node.operand.accept(this);
6063+
// `await e` first checks if the runtime type of `e` is a subtype of
6064+
// `Future<T>` where `T = flatten(S)` and `S` is the static type of `e`.
6065+
// (The type `Future<T>` is helpfully precomputed by the CFE and stored in
6066+
// [AwaitExpression.runtimeCheckType].)
6067+
//
6068+
// If the check succeeds (or if no `runtimeCheckType` is provided), we await
6069+
// `e` directly.
6070+
//
6071+
// If the check fails, we must await `Future.value(e)` instead.
6072+
//
6073+
// See https://github.com/dart-lang/sdk/issues/49396 for details.
6074+
6075+
final operand = node.operand;
6076+
operand.accept(this);
60646077
HInstruction awaited = pop();
6065-
// TODO(herhut): Improve this type.
6066-
push(HAwait(awaited, _abstractValueDomain.dynamicType)
6067-
..sourceInformation = _sourceInformationBuilder.buildAwait(node));
6078+
final runtimeCheckType = node.runtimeCheckType;
6079+
final sourceInformation = _sourceInformationBuilder.buildAwait(node);
6080+
6081+
void checkType() {
6082+
// TODO(fishythefish): Can we get rid of the redundancy with the type
6083+
// checks in async_patch?
6084+
_pushIsTest(runtimeCheckType!, awaited, sourceInformation);
6085+
}
6086+
6087+
void pushAwait(HInstruction expression) {
6088+
// TODO(herhut): Improve this type.
6089+
push(HAwait(expression, _abstractValueDomain.dynamicType)
6090+
..sourceInformation = sourceInformation);
6091+
}
6092+
6093+
void awaitUnwrapped() {
6094+
pushAwait(awaited);
6095+
}
6096+
6097+
void awaitWrapped() {
6098+
final constructor = _commonElements.futureValueConstructor!;
6099+
final arguments = [awaited];
6100+
// If [runtimeCheckType] exists, it is guaranteed to be `Future<T>`, and
6101+
// we want to call `Future<T>.value`.
6102+
final typeArgument = _elementMap.getDartType(
6103+
(runtimeCheckType as ir.InterfaceType).typeArguments.single);
6104+
final typeArguments = [typeArgument];
6105+
_addTypeArguments(arguments, typeArguments, sourceInformation);
6106+
final instanceType = _commonElements.futureType(typeArgument);
6107+
_addImplicitInstantiation(instanceType);
6108+
_pushStaticInvocation(constructor, arguments,
6109+
_typeInferenceMap.getReturnTypeOf(constructor), typeArguments,
6110+
sourceInformation: sourceInformation, instanceType: instanceType);
6111+
_removeImplicitInstantiation(instanceType);
6112+
pushAwait(pop());
6113+
}
6114+
6115+
if (runtimeCheckType == null) {
6116+
awaitUnwrapped();
6117+
} else {
6118+
SsaBranchBuilder(this)
6119+
.handleConditional(checkType, awaitUnwrapped, awaitWrapped);
6120+
}
60686121
}
60696122

60706123
@override

tests/web/await_non_future_test.dart

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
class A<T> {
6+
factory A() = B<T>;
7+
}
8+
9+
class B<T> implements A<T> {}
10+
11+
main() async {
12+
await A<void>();
13+
}

0 commit comments

Comments
 (0)