Make type variable constraint generation conventions uniform. This CL changes the CFE's TypeConstraintGatherer._isNullabilityAwareSubtypeMatch method so that it is responsible for restoring the constraint state if there is no match, making it consistent with the contraint gathering methods in the analyzer and _fe_analyzer_shared. This made it possible to remove much of the calls to state restoring logic that _isNullabilityAwareSubtypeMatch previously had to do after making recursive calls to itself, as well as a lot of state restoring logic in _fe_analyzer_shared. It also made it possible to eliminate _tryNullabilityAwareSubtypeMatch from the CFE (since _isNullabilityAwareSubtypeMatch now has the same behavior). Making this change now should hopefully simplify the remaining steps in sharing type variable constraint generation logic, since it will no longer be necessary to adjust state restoring logic when moving code between the CFE and _fe_analyzer_shared. I also took the liberty of rewriting some of the documentation comments to try to clarify the new conventions. In the process I also discovered several instances of unnecessary state restoring logic in the analyzer; I'll make a separate CL to clean those up (and adjust the analyzer documentation too). Change-Id: If74c8be06f1d53f61d109e5ea2a8526d5cbcd347 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/388265 Commit-Queue: Chloe Stefantsova <cstefantsova@google.com> Reviewed-by: Chloe Stefantsova <cstefantsova@google.com> Auto-Submit: Paul Berry <paulberry@google.com>
diff --git a/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer_operations.dart b/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer_operations.dart index f7a47b8..1f3ad2b 100644 --- a/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer_operations.dart +++ b/pkg/_fe_analyzer_shared/lib/src/type_inference/type_analyzer_operations.dart
@@ -898,23 +898,18 @@ List<TypeStructure>? getTypeArgumentsAsInstanceOf( TypeDeclarationType type, TypeDeclaration typeDeclaration); - /// Matches [p] against [q] as a subtype against supertype and returns true if - /// [p] and [q] are both function types, and [p] is a subtype of [q] under - /// some constraints imposed on type parameters occurring in [q]; false if - /// both [p] and [q] are function types, but [p] can't possibly be a subtype - /// of [q] under any constraints; and null otherwise. + /// Matches [p] against [q]. + /// + /// If [p] and [q] are both non-generic function types, and [p] is a subtype + /// of [q] under some constraints, the constraints making the relation + /// possible are recorded, and `true` is returned. Otherwise, the constraint + /// state is unchanged (or rolled back using [restoreState]), and `null` is + /// returned. /// /// An invariant of the type inference is that only [p] or [q] may be a /// schema (in other words, may contain the unknown type `_`); the other must /// be simply a type. If [leftSchema] is `true`, [p] may contain `_`; if it is /// `false`, [q] may contain `_`. - /// - /// As the generator computes the constraints making the relation possible, it - /// changes its internal state. The current state of the generator can be - /// obtained by [currentState], and the generator can be restored to a state - /// via [restoreState]. All of the shared constraint generation methods are - /// supposed to restore the generator to the prior state in case of a - /// mismatch, taking that responsibility away from the caller. bool? performSubtypeConstraintGenerationForFunctionTypes( TypeStructure p, TypeStructure q, {required bool leftSchema, required AstNode? astNodeForTesting}) { @@ -943,7 +938,6 @@ if (!performSubtypeConstraintGenerationInternal( p.returnType, q.returnType, leftSchema: leftSchema, astNodeForTesting: astNodeForTesting)) { - restoreState(state); return null; } for (int i = 0; i < q.positionalParameterTypes.length; ++i) { @@ -973,7 +967,6 @@ if (!performSubtypeConstraintGenerationInternal( p.returnType, q.returnType, leftSchema: leftSchema, astNodeForTesting: astNodeForTesting)) { - restoreState(state); return null; } for (int i = 0; i < p.positionalParameterTypes.length; ++i) { @@ -1048,23 +1041,18 @@ return null; } - /// Matches [p] against [q] as a subtype against supertype and returns true if - /// [p] and [q] are both FutureOr, with or without nullability suffixes as - /// defined by [enableDiscrepantObliviousnessOfNullabilitySuffixOfFutureOr], - /// and [p] is a subtype of [q] under some constraints imposed on type - /// parameters occurring in [q], and false otherwise. + /// Matches [p] against [q]. + /// + /// If [q] is of the form `FutureOr<q0>` for some `q0`, and [p] is a subtype + /// of [q] under some constraints, the constraints making the relation + /// possible are recorded, and `true` is returned. Otherwise, the constraint + /// state is unchanged (or rolled back using [restoreState]), and `false` is + /// returned. /// /// An invariant of the type inference is that only [p] or [q] may be a /// schema (in other words, may contain the unknown type `_`); the other must /// be simply a type. If [leftSchema] is `true`, [p] may contain `_`; if it is /// `false`, [q] may contain `_`. - /// - /// As the generator computes the constraints making the relation possible, - /// it changes its internal state. The current state of the generator can be - /// obtained by [currentState], and the generator can be restored to a state - /// via [restoreState]. All of the shared constraint generation methods are - /// supposed to restore the generator to the prior state in case of a - /// mismatch, taking that responsibility away from the caller. bool performSubtypeConstraintGenerationForFutureOr( TypeStructure p, TypeStructure q, {required bool leftSchema, required AstNode? astNodeForTesting}) { @@ -1083,7 +1071,6 @@ leftSchema: leftSchema, astNodeForTesting: astNodeForTesting)) { return true; } - restoreState(state); } // Or if `P` is a subtype match for `Future<Q0>` under non-empty @@ -1095,14 +1082,12 @@ if (isMatchWithFuture && matchWithFutureAddsConstraints) { return true; } - restoreState(state); // Or if `P` is a subtype match for `Q0` under constraint set `C`. if (performSubtypeConstraintGenerationInternal(p, q0, leftSchema: leftSchema, astNodeForTesting: astNodeForTesting)) { return true; } - restoreState(state); // Or if `P` is a subtype match for `Future<Q0>` under empty // constraint set `C`. @@ -1114,24 +1099,22 @@ return false; } - /// Matches [p] against [q] as a subtype against supertype and returns true if - /// [p] and [q] are both type declaration types as defined by the enum - /// [TypeDeclarationKind], and [p] is a subtype of [q] under some constraints - /// imposed on type parameters occurring in [q]; false if both [p] and [q] are - /// type declaration types, but [p] can't possibly be a subtype of [q] under - /// any constraints; and null otherwise. + /// Matches [p] against [q] as a subtype against supertype. + /// + /// If [p] and [q] are both type declaration types, then: + /// + /// - If [p] is a subtype of [q] under some constraints, the constraints + /// making the relation possible are recorded, and `true` is returned. + /// - Otherwise, the constraint state is unchanged (or rolled back using + /// [restoreState]), and `false` is returned. + /// + /// Otherwise (either [p] or [q] is not a type declaration type), the + /// constraint state is unchanged, and `null` is returned. /// /// An invariant of the type inference is that only [p] or [q] may be a /// schema (in other words, may contain the unknown type `_`); the other must /// be simply a type. If [leftSchema] is `true`, [p] may contain `_`; if it is /// `false`, [q] may contain `_`. - /// - /// As the generator computes the constraints making the relation possible, it - /// changes its internal state. The current state of the generator can be - /// obtained by [currentState], and the generator can be restored to a state - /// via [restoreState]. All of the shared constraint generation methods are - /// supposed to restore the generator to the prior state in case of a - /// mismatch, taking that responsibility away from the caller. bool? performSubtypeConstraintGenerationForTypeDeclarationTypes( TypeStructure p, TypeStructure q, {required bool leftSchema, required AstNode? astNodeForTesting}) { @@ -1174,6 +1157,14 @@ } } + /// Implementation backing [performSubtypeConstraintGenerationLeftSchema] and + /// [performSubtypeConstraintGenerationRightSchema]. + /// + /// If [p] is a subtype of [q] under some constraints, the constraints making + /// the relation possible are recorded, and `true` is returned. Otherwise, + /// the constraint state is unchanged (or rolled back using [restoreState]), + /// and `false` is returned. + /// /// [performSubtypeConstraintGenerationInternal] should be implemented by /// concrete classes implementing [TypeConstraintGenerator]. The /// implementations of [performSubtypeConstraintGenerationLeftSchema] and @@ -1201,9 +1192,8 @@ /// As the generator computes the constraints making the relation possible, /// it changes its internal state. The current state of the generator can be /// obtained by [currentState], and the generator can be restored to a state - /// via [restoreState]. All of the shared constraint generation methods are - /// supposed to restore the generator to the prior state in case of a - /// mismatch, taking that responsibility away from the caller. + /// via [restoreState]. If this method returns `false`, it restores the state, + /// so it is not necessary for the caller to do so. /// /// The algorithm for subtype constraint generation is described in /// https://github.com/dart-lang/language/blob/main/resources/type-system/inference.md#subtype-constraint-generation @@ -1219,9 +1209,8 @@ /// As the generator computes the constraints making the relation possible, /// it changes its internal state. The current state of the generator can be /// obtained by [currentState], and the generator can be restored to a state - /// via [restoreState]. All of the shared constraint generation methods are - /// supposed to restore the generator to the prior state in case of a - /// mismatch, taking that responsibility away from the caller. + /// via [restoreState]. If this method returns `false`, it restores the state, + /// so it is not necessary for the caller to do so. /// /// The algorithm for subtype constraint generation is described in /// https://github.com/dart-lang/language/blob/main/resources/type-system/inference.md#subtype-constraint-generation @@ -1271,6 +1260,13 @@ return true; } + /// Matches [p] against [q], assuming both [p] and [q] are both type + /// declaration types that refer to different type declarations. + /// + /// If [p] is a subtype of [q] under some constraints, the constraints making + /// the relation possible are recorded, and `true` is returned. Otherwise, + /// the constraint state is unchanged (or rolled back using [restoreState]), + /// and `false` is returned. bool _interfaceTypes(TypeStructure p, TypeStructure q, bool leftSchema, {required AstNode? astNodeForTesting}) { if (p.nullabilitySuffix != NullabilitySuffix.none) {
diff --git a/pkg/front_end/lib/src/type_inference/type_constraint_gatherer.dart b/pkg/front_end/lib/src/type_inference/type_constraint_gatherer.dart index 2cc1e045..c5f45ee 100644 --- a/pkg/front_end/lib/src/type_inference/type_constraint_gatherer.dart +++ b/pkg/front_end/lib/src/type_inference/type_constraint_gatherer.dart
@@ -123,7 +123,7 @@ /// a subtype of [type] under any set of constraints. bool tryConstrainLower(DartType type, DartType bound, {required TreeNode? treeNodeForTesting}) { - return _tryNullabilityAwareSubtypeMatch(bound, type, + return _isNullabilityAwareSubtypeMatch(bound, type, constrainSupertype: true, treeNodeForTesting: treeNodeForTesting); } @@ -133,7 +133,7 @@ /// a subtype of [bound] under any set of constraints. bool tryConstrainUpper(DartType type, DartType bound, {required TreeNode? treeNodeForTesting}) { - return _tryNullabilityAwareSubtypeMatch(type, bound, + return _isNullabilityAwareSubtypeMatch(type, bound, constrainSupertype: false, treeNodeForTesting: treeNodeForTesting); } @@ -156,34 +156,6 @@ return isMatch; } - /// Tries to match [subtype] against [supertype]. - /// - /// If the match succeeds, the member returns true, and the resulting type - /// constraints are recorded for later use by [computeConstraints]. If the - /// match fails, the member returns false, and the set of type constraints is - /// unchanged. - /// - /// In contrast with [_tryNullabilityObliviousSubtypeMatch], this method - /// distinguishes between cases when the type parameters to constraint occur - /// in [subtype] and in [supertype]. If [constrainSupertype] is true, the - /// type parameters to constrain occur in [supertype]; otherwise, they occur - /// in [subtype]. If one type contains the type parameters to constrain, the - /// other one isn't allowed to contain them. The type that contains the type - /// parameters isn't allowed to also contain [UnknownType], that is, to be a - /// type schema. - bool _tryNullabilityAwareSubtypeMatch(DartType subtype, DartType supertype, - {required bool constrainSupertype, - required TreeNode? treeNodeForTesting}) { - int baseConstraintCount = _protoConstraints.length; - bool isMatch = _isNullabilityAwareSubtypeMatch(subtype, supertype, - constrainSupertype: constrainSupertype, - treeNodeForTesting: treeNodeForTesting); - if (!isMatch) { - _protoConstraints.length = baseConstraintCount; - } - return isMatch; - } - /// Add constraint: [lower] <: [parameter] <: TOP. void _constrainParameterLower(StructuralParameter parameter, DartType lower, {required TreeNode? treeNodeForTesting}) { @@ -367,10 +339,10 @@ /// Matches [p] against [q] as a subtype against supertype. /// - /// Returns true if [p] is a subtype of [q] under some constraints, and false - /// otherwise. The constraints making the relation possible are recorded to - /// [_protoConstraints]. It is the responsibility of the caller to cleanup - /// [_protoConstraints] in case [p] can't be a subtype of [q]. + /// If [p] is a subtype of [q] under some constraints, the constraints making + /// the relation possible are recorded to [_protoConstraints], and `true` is + /// returned. Otherwise, [_protoConstraints] is left unchanged (or rolled + /// back), and `false` is returned. /// /// If [constrainSupertype] is true, the type parameters to constrain occur in /// [supertype]; otherwise, they occur in [subtype]. If one type contains the @@ -505,7 +477,6 @@ treeNodeForTesting: treeNodeForTesting)) { return true; } - _protoConstraints.length = baseConstraintCount; if ((p is SharedDynamicTypeStructure || p is SharedVoidTypeStructure) && _isNullabilityAwareSubtypeMatch( @@ -514,7 +485,6 @@ treeNodeForTesting: treeNodeForTesting)) { return true; } - _protoConstraints.length = baseConstraintCount; bool isMatchWithRawQ = _isNullabilityAwareSubtypeMatch(p, rawQ, constrainSupertype: constrainSupertype, @@ -524,7 +494,6 @@ if (isMatchWithRawQ && matchWithRawQAddsConstraints) { return true; } - _protoConstraints.length = baseConstraintCount; if (_isNullabilityAwareSubtypeMatch( p, typeOperations.nullType.unwrapTypeView(), @@ -532,12 +501,10 @@ treeNodeForTesting: treeNodeForTesting)) { return true; } - _protoConstraints.length = baseConstraintCount; if (isMatchWithRawQ && !matchWithRawQAddsConstraints) { return true; } - _protoConstraints.length = baseConstraintCount; } // If P is FutureOr<P0> the match holds under constraint set C1 + C2: @@ -616,22 +583,18 @@ // already eliminated the case that X is a variable in L. if (p is TypeParameterType) { // Coverage-ignore-block(suite): Not run. - final int baseConstraintCount = _protoConstraints.length; if (_isNullabilityAwareSubtypeMatch(p.bound, q, constrainSupertype: constrainSupertype, treeNodeForTesting: treeNodeForTesting)) { return true; } - _protoConstraints.length = baseConstraintCount; } else if (p is StructuralParameterType) { // Coverage-ignore-block(suite): Not run. - final int baseConstraintCount = _protoConstraints.length; if (_isNullabilityAwareSubtypeMatch(p.bound, q, constrainSupertype: constrainSupertype, treeNodeForTesting: treeNodeForTesting)) { return true; } - _protoConstraints.length = baseConstraintCount; } bool? constraintGenerationResult = @@ -774,6 +737,7 @@ } } if (sameNames) { + final int baseConstraintCount = _protoConstraints.length; bool isMatch = true; for (int i = 0; isMatch && i < p.positional.length; i++) { isMatch = isMatch && @@ -788,6 +752,8 @@ treeNodeForTesting: treeNodeForTesting); } if (isMatch) return true; + // Coverage-ignore(suite): Not run. + _protoConstraints.length = baseConstraintCount; } }
diff --git a/pkg/front_end/test/spell_checking_list_code.txt b/pkg/front_end/test/spell_checking_list_code.txt index d0960b6..953e184 100644 --- a/pkg/front_end/test/spell_checking_list_code.txt +++ b/pkg/front_end/test/spell_checking_list_code.txt
@@ -121,6 +121,7 @@ awaiting awaits b +backing backlog backping backstop