From 71efc19cc751da67b7eeeb3988bca0685bfb7c3e Mon Sep 17 00:00:00 2001 From: Alexander Markov Date: Fri, 30 Aug 2024 13:55:34 +0000 Subject: [PATCH] [vm/aot,tfa] Remove support for legacy types from TFA In addition, with sound null safety there is no difference in subtyping semantic between 'is' and 'as' type checks, so SubtypeTestKind is also removed. TEST=ci Change-Id: Id75ef6a93b115c44b844ef0a32dc8dca5cc61861 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/382900 Reviewed-by: Martin Kustermann Commit-Queue: Alexander Markov --- .../transformations/type_flow/summary.dart | 5 +- .../type_flow/summary_collector.dart | 32 ++----- .../lib/transformations/type_flow/types.dart | 89 ++++++------------- .../lib/transformations/type_flow/utils.dart | 1 - 4 files changed, 33 insertions(+), 94 deletions(-) diff --git a/pkg/vm/lib/transformations/type_flow/summary.dart b/pkg/vm/lib/transformations/type_flow/summary.dart index 17e3cf8fd36..1c06356f14f 100644 --- a/pkg/vm/lib/transformations/type_flow/summary.dart +++ b/pkg/vm/lib/transformations/type_flow/summary.dart @@ -635,7 +635,6 @@ class TypeCheck extends Statement { final TreeNode node; final Type staticType; - final SubtypeTestKind kind; // 'isTestedOnlyOnCheckedEntryPoint' is whether or not this parameter's type-check will // occur on the "checked" entrypoint in the VM but will be skipped on @@ -648,7 +647,7 @@ class TypeCheck extends Statement { bool alwaysPass = true; bool alwaysFail = true; - TypeCheck(this.arg, this.type, this.node, this.staticType, this.kind) + TypeCheck(this.arg, this.type, this.node, this.staticType) : isTestedOnlyOnCheckedEntryPoint = node is VariableDeclaration && !node.isCovariantByDeclaration; @@ -681,7 +680,7 @@ class TypeCheck extends Statement { // guarantee that it will pass. pass = false; } else if (checkType is RuntimeType) { - pass = argType.isSubtypeOfRuntimeType(typeHierarchy, checkType, kind); + pass = argType.isSubtypeOfRuntimeType(typeHierarchy, checkType); argType = argType.intersection( typeHierarchy.fromStaticType(checkType.representedTypeRaw, true), typeHierarchy); diff --git a/pkg/vm/lib/transformations/type_flow/summary_collector.dart b/pkg/vm/lib/transformations/type_flow/summary_collector.dart index 877b9c2532c..0e1618a3193 100644 --- a/pkg/vm/lib/transformations/type_flow/summary_collector.dart +++ b/pkg/vm/lib/transformations/type_flow/summary_collector.dart @@ -1254,14 +1254,10 @@ class SummaryCollector extends RecursiveResultVisitor { } } - TypeCheck _typeCheck(TypeExpr value, DartType type, TreeNode node, - [SubtypeTestKind kind = SubtypeTestKind.Subtype]) { + TypeCheck _typeCheck(TypeExpr value, DartType type, TreeNode node) { final TypeExpr runtimeType = _translator.translate(type); - final bool canBeNull = (kind == SubtypeTestKind.IsTest) - ? _canBeNullAfterSuccessfulIsCheck(type) - : true; - final typeCheck = new TypeCheck(value, runtimeType, node, - _typesBuilder.fromStaticType(type, canBeNull), kind); + final typeCheck = new TypeCheck( + value, runtimeType, node, _typesBuilder.fromStaticType(type, true)); typeCheck.condition = _currentCondition; _summary.add(typeCheck); return typeCheck; @@ -1331,22 +1327,6 @@ class SummaryCollector extends RecursiveResultVisitor { return narrow; } - bool _canBeNullAfterSuccessfulIsCheck(DartType type) { - // 'x is type' can succeed for null if type is - // - a top type (dynamic, void, Object? or Object*) - // - nullable (including Null) - // - a type parameter (it can be instantiated with Null) - // - legacy Never - // - a FutureOr of the above - final nullability = type.nullability; - return _environment.isTop(type) || - nullability == Nullability.nullable || - type is TypeParameterType || - (type is NeverType && nullability == Nullability.legacy) || - (type is FutureOrType && - _canBeNullAfterSuccessfulIsCheck(type.typeArgument)); - } - TypeExpr _makeNarrowNotNull(TreeNode node, TypeExpr arg) { assert(node is NullCheck || node is EqualsNull); if (arg is NarrowNotNull) { @@ -1631,8 +1611,7 @@ class SummaryCollector extends RecursiveResultVisitor { } else if (node is IsExpression && node.operand is VariableGet) { // Handle 'x is T', where x is a variable. final operand = node.operand as VariableGet; - final TypeCheck typeCheck = - _typeCheck(_visit(operand), node.type, node, SubtypeTestKind.IsTest); + final TypeCheck typeCheck = _typeCheck(_visit(operand), node.type, node); isTests[node] = typeCheck; final int varIndex = _variablesInfo.varIndex[operand.variable]!; if (!_aggregateVariable[varIndex]) { @@ -1785,8 +1764,7 @@ class SummaryCollector extends RecursiveResultVisitor { TypeExpr visitIsExpression(IsExpression node) { final operandNode = node.operand; final TypeExpr operand = _visit(operandNode); - final TypeCheck typeCheck = - _typeCheck(operand, node.type, node, SubtypeTestKind.IsTest); + final TypeCheck typeCheck = _typeCheck(operand, node.type, node); isTests[node] = typeCheck; return _boolType; } diff --git a/pkg/vm/lib/transformations/type_flow/types.dart b/pkg/vm/lib/transformations/type_flow/types.dart index 9a329c3bc83..7ffdd14643e 100644 --- a/pkg/vm/lib/transformations/type_flow/types.dart +++ b/pkg/vm/lib/transformations/type_flow/types.dart @@ -311,13 +311,6 @@ abstract class TypeExpr { Type getComputedType(List types); } -/// Kind of a subtype test: subtype/cast/'as' test or instance check/'is' test. -/// There is a subtle difference in how these tests handle null value. -enum SubtypeTestKind { - Subtype, - IsTest, -} - /// Base class for types inferred by the type flow analysis. /// [Type] describes a specific set of values (Dart instances) and does not /// directly correspond to a Dart type. @@ -337,8 +330,8 @@ abstract class Type extends TypeExpr { // Returns 'true' if this type will definitely pass a runtime type-check // against 'runtimeType'. Returns 'false' if the test might fail (e.g. due to // an approximation). - bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, - RuntimeType runtimeType, SubtypeTestKind kind); + bool isSubtypeOfRuntimeType( + TypeHierarchy typeHierarchy, RuntimeType runtimeType); @override Type getComputedType(List types) => this; @@ -426,8 +419,7 @@ class EmptyType extends Type { @override Type intersection(Type other, TypeHierarchy typeHierarchy) => this; - bool isSubtypeOfRuntimeType( - TypeHierarchy typeHierarchy, RuntimeType other, SubtypeTestKind kind) { + bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, RuntimeType other) { return true; } } @@ -461,26 +453,11 @@ class NullableType extends Type { @override bool isSubtypeOf(TFClass cls) => baseType.isSubtypeOf(cls); - bool isSubtypeOfRuntimeType( - TypeHierarchy typeHierarchy, RuntimeType other, SubtypeTestKind kind) { - switch (kind) { - case SubtypeTestKind.Subtype: - if (other.nullability == Nullability.nonNullable) { - return false; - } - break; - case SubtypeTestKind.IsTest: - if (other.nullability != Nullability.nullable) { - final rhs = other._type; - if (!(rhs is InterfaceType && - rhs.nullability == Nullability.legacy && - rhs.classNode == typeHierarchy.coreTypes.objectClass)) { - return false; - } - } - break; + bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, RuntimeType other) { + if (other.nullability == Nullability.nonNullable) { + return false; } - return baseType.isSubtypeOfRuntimeType(typeHierarchy, other, kind); + return baseType.isSubtypeOfRuntimeType(typeHierarchy, other); } @override @@ -558,8 +535,7 @@ class AnyInstanceType extends Type { return other; } - bool isSubtypeOfRuntimeType( - TypeHierarchy typeHierarchy, RuntimeType other, SubtypeTestKind kind) { + bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, RuntimeType other) { final rhs = other._type; return (rhs is DynamicType) || (rhs is VoidType) || @@ -635,9 +611,8 @@ class SetType extends Type { bool isSubtypeOf(TFClass cls) => types.every((ConcreteType t) => t.isSubtypeOf(cls)); - bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, RuntimeType other, - SubtypeTestKind kind) => - types.every((t) => t.isSubtypeOfRuntimeType(typeHierarchy, other, kind)); + bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, RuntimeType other) => + types.every((t) => t.isSubtypeOfRuntimeType(typeHierarchy, other)); @override int get order => TypeOrder.Set.index; @@ -837,8 +812,7 @@ class ConeType extends Type { @override bool isSubtypeOf(TFClass cls) => this.cls.isSubtypeOf(cls); - bool isSubtypeOfRuntimeType( - TypeHierarchy typeHierarchy, RuntimeType other, SubtypeTestKind kind) { + bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, RuntimeType other) { final rhs = other._type; if (rhs is DynamicType || rhs is VoidType) return true; if (rhs is InterfaceType) { @@ -1211,8 +1185,8 @@ class ConcreteType extends Type implements Comparable { @override bool isSubtypeOf(TFClass other) => cls.isSubtypeOf(other); - bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, - RuntimeType runtimeType, SubtypeTestKind kind) { + bool isSubtypeOfRuntimeType( + TypeHierarchy typeHierarchy, RuntimeType runtimeType) { final rhs = runtimeType._type; if (rhs is DynamicType || rhs is VoidType) return true; if (rhs is InterfaceType) { @@ -1252,7 +1226,7 @@ class ConcreteType extends Type implements Comparable { } assert(ta is RuntimeType); if (!ta.isSubtypeOfRuntimeType( - typeHierarchy, runtimeType.typeArgs![i], SubtypeTestKind.Subtype)) { + typeHierarchy, runtimeType.typeArgs![i])) { return false; } } @@ -1272,10 +1246,9 @@ class ConcreteType extends Type implements Comparable { final RuntimeType lhs = typeArg is RuntimeType ? typeArg : RuntimeType(DynamicType(), null); return lhs.isSubtypeOfRuntimeType( - typeHierarchy, runtimeType.typeArgs![0], SubtypeTestKind.Subtype); + typeHierarchy, runtimeType.typeArgs![0]); } else { - return isSubtypeOfRuntimeType( - typeHierarchy, runtimeType.typeArgs![0], SubtypeTestKind.Subtype); + return isSubtypeOfRuntimeType(typeHierarchy, runtimeType.typeArgs![0]); } } return false; @@ -1480,9 +1453,6 @@ class RuntimeType extends Type { if (thisNullability == Nullability.nullable || nullability == Nullability.nullable) { result = Nullability.nullable; - } else if (thisNullability == Nullability.legacy || - nullability == Nullability.legacy) { - result = Nullability.legacy; } else { result = Nullability.nonNullable; } @@ -1584,11 +1554,8 @@ class RuntimeType extends Type { Class? getConcreteClass(TypeHierarchy typeHierarchy) => throw "ERROR: RuntimeType does not support getConcreteClass."; - bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, - RuntimeType runtimeType, SubtypeTestKind kind) { - if (kind != SubtypeTestKind.Subtype) { - throw 'RuntimeType could be only tested for subtyping.'; - } + bool isSubtypeOfRuntimeType( + TypeHierarchy typeHierarchy, RuntimeType runtimeType) { final rhs = runtimeType._type; if (_type.nullability == Nullability.nullable && rhs.nullability == Nullability.nonNullable) { @@ -1608,15 +1575,15 @@ class RuntimeType extends Type { if (_type is InterfaceType) { Class thisClass = _type.classNode; if (thisClass == typeHierarchy.coreTypes.futureClass) { - return typeArgs![0].isSubtypeOfRuntimeType( - typeHierarchy, runtimeType.typeArgs![0], SubtypeTestKind.Subtype); + return typeArgs![0] + .isSubtypeOfRuntimeType(typeHierarchy, runtimeType.typeArgs![0]); } else { return isSubtypeOfRuntimeType( - typeHierarchy, runtimeType.typeArgs![0], SubtypeTestKind.Subtype); + typeHierarchy, runtimeType.typeArgs![0]); } } else if (_type is FutureOrType) { - return typeArgs![0].isSubtypeOfRuntimeType( - typeHierarchy, runtimeType.typeArgs![0], SubtypeTestKind.Subtype); + return typeArgs![0] + .isSubtypeOfRuntimeType(typeHierarchy, runtimeType.typeArgs![0]); } } @@ -1655,8 +1622,8 @@ class RuntimeType extends Type { assert(usableTypeArgs.length - interfaceOffset >= runtimeType.numImmediateTypeArgs); for (int i = 0; i < runtimeType.numImmediateTypeArgs; ++i) { - if (!usableTypeArgs[interfaceOffset + i].isSubtypeOfRuntimeType( - typeHierarchy, runtimeType.typeArgs![i], SubtypeTestKind.Subtype)) { + if (!usableTypeArgs[interfaceOffset + i] + .isSubtypeOfRuntimeType(typeHierarchy, runtimeType.typeArgs![i])) { return false; } } @@ -1709,11 +1676,7 @@ class UnknownType extends Type { throw "ERROR: UnknownType does not support intersection with ${other.runtimeType}"; } - bool isSubtypeOfRuntimeType( - TypeHierarchy typeHierarchy, RuntimeType other, SubtypeTestKind kind) { - if (kind != SubtypeTestKind.Subtype) { - throw 'UnknownType could be only tested for subtyping.'; - } + bool isSubtypeOfRuntimeType(TypeHierarchy typeHierarchy, RuntimeType other) { final rhs = other._type; return (rhs is DynamicType) || (rhs is VoidType) || diff --git a/pkg/vm/lib/transformations/type_flow/utils.dart b/pkg/vm/lib/transformations/type_flow/utils.dart index 23e8dbe094a..60ec8279eba 100644 --- a/pkg/vm/lib/transformations/type_flow/utils.dart +++ b/pkg/vm/lib/transformations/type_flow/utils.dart @@ -378,7 +378,6 @@ class UnionFind { } const nullabilitySuffix = { - Nullability.legacy: '*', Nullability.nullable: '?', Nullability.undetermined: '', Nullability.nonNullable: '',