From fcb7d76e74a5fe9340208d6914a086c54bc18a56 Mon Sep 17 00:00:00 2001 From: Kathy Gray Date: Tue, 28 Oct 2025 15:17:17 +0000 Subject: [PATCH 1/2] RequirementMachine: Adding shape abstractions Adding abstractions to check terms for shape symbol and remove the shape symbol from the end of the sequence of symbols, rather than manually manipulating the end() sequence externally. --- .../GenericSignatureQueries.cpp | 6 +++--- lib/AST/RequirementMachine/InterfaceType.cpp | 8 ++++---- .../RequirementMachine/RequirementBuilder.cpp | 11 +++++----- lib/AST/RequirementMachine/RewriteSystem.cpp | 2 +- lib/AST/RequirementMachine/Term.cpp | 20 +++++++++++++++++++ lib/AST/RequirementMachine/Term.h | 6 ++++++ 6 files changed, 39 insertions(+), 14 deletions(-) diff --git a/lib/AST/RequirementMachine/GenericSignatureQueries.cpp b/lib/AST/RequirementMachine/GenericSignatureQueries.cpp index 66d18b5fce70c..0d4c758d8160b 100644 --- a/lib/AST/RequirementMachine/GenericSignatureQueries.cpp +++ b/lib/AST/RequirementMachine/GenericSignatureQueries.cpp @@ -755,7 +755,7 @@ RequirementMachine::getReducedShapeTerm(Type type) const { // Get the term T', which is the reduced shape of T. if (term.size() != 2 || term[0].getKind() != Symbol::Kind::GenericParam || - term[1].getKind() != Symbol::Kind::Shape) { + !term.hasShape()) { ABORT([&](auto &out) { out << "Invalid reduced shape\n"; out << "Type: " << type << "\n"; @@ -763,8 +763,8 @@ RequirementMachine::getReducedShapeTerm(Type type) const { }); } - MutableTerm reducedTerm(term.begin(), term.end() - 1); - return reducedTerm; + term.removeShape(); + return term; } Type RequirementMachine::getReducedShape(Type type, diff --git a/lib/AST/RequirementMachine/InterfaceType.cpp b/lib/AST/RequirementMachine/InterfaceType.cpp index 56216ef16653c..600da8a6582ce 100644 --- a/lib/AST/RequirementMachine/InterfaceType.cpp +++ b/lib/AST/RequirementMachine/InterfaceType.cpp @@ -424,7 +424,7 @@ RewriteContext::getRelativeTermForType(CanType typeWitness, // Get the substitution S corresponding to τ_0_n. unsigned index = getGenericParamIndex(typeWitness->getRootGenericParam()); result = MutableTerm(substitutions[index]); - ASSERT(result.back().getKind() != Symbol::Kind::Shape); + ASSERT(!result.hasShape()); // If the substitution is a term consisting of a single protocol symbol // [P], save P for later. @@ -485,7 +485,7 @@ Type PropertyMap::getTypeFromSubstitutionSchema( auto substitution = substitutions[index]; bool isShapePosition = (pos == TypePosition::Shape); - bool isShapeTerm = (substitution.back() == Symbol::forShape(Context)); + bool isShapeTerm = substitution.hasShape(); if (isShapePosition != isShapeTerm) { ABORT([&](auto &out) { out << "Shape vs. type mixup\n\n"; @@ -504,8 +504,8 @@ Type PropertyMap::getTypeFromSubstitutionSchema( // Undo the thing where the count type of a PackExpansionType // becomes a shape term. if (isShapeTerm) { - MutableTerm mutTerm(substitution.begin(), substitution.end() - 1); - substitution = Term::get(mutTerm, Context); + MutableTerm noShape = substitution.termWithoutShape(); + substitution = Term::get(noShape, Context); } // Prepend the prefix of the lookup key to the substitution. diff --git a/lib/AST/RequirementMachine/RequirementBuilder.cpp b/lib/AST/RequirementMachine/RequirementBuilder.cpp index 21da0433bbf4d..fb6b2d89f6dbb 100644 --- a/lib/AST/RequirementMachine/RequirementBuilder.cpp +++ b/lib/AST/RequirementMachine/RequirementBuilder.cpp @@ -257,11 +257,10 @@ void RequirementBuilder::addRequirementRules(ArrayRef rules) { ASSERT(rule.getLHS().back().getKind() != Symbol::Kind::Protocol); - if (constraintTerm.back().getKind() == Symbol::Kind::Shape) { - ASSERT(rule.getRHS().back().getKind() == Symbol::Kind::Shape); + if (constraintTerm.hasShape()) { + ASSERT(rule.getRHS().hasShape()); // Strip off the shape symbol from the constraint term. - constraintTerm = MutableTerm(constraintTerm.begin(), - constraintTerm.end() - 1); + constraintTerm.removeShape(); } if (constraintTerm.front().getKind() == Symbol::Kind::PackElement) { @@ -332,10 +331,10 @@ void RequirementBuilder::processConnectedComponents() { for (auto &pair : Components) { MutableTerm subjectTerm(pair.first); RequirementKind kind; - if (subjectTerm.back().getKind() == Symbol::Kind::Shape) { + if (subjectTerm.hasShape()) { kind = RequirementKind::SameShape; // Strip off the shape symbol from the subject term. - subjectTerm = MutableTerm(subjectTerm.begin(), subjectTerm.end() - 1); + subjectTerm.removeShape(); } else { kind = RequirementKind::SameType; if (subjectTerm.front().getKind() == Symbol::Kind::PackElement) { diff --git a/lib/AST/RequirementMachine/RewriteSystem.cpp b/lib/AST/RequirementMachine/RewriteSystem.cpp index ac4dda392d95a..37789398f792b 100644 --- a/lib/AST/RequirementMachine/RewriteSystem.cpp +++ b/lib/AST/RequirementMachine/RewriteSystem.cpp @@ -634,7 +634,7 @@ void RewriteSystem::verifyRewriteRules(ValidityPolicy policy) const { if (rhs.size() == 1 && rhs[0].getKind() == Symbol::Kind::Shape) { // We can have a rule like T.[shape] => [shape]. - ASSERT_RULE(lhs.back().getKind() == Symbol::Kind::Shape); + ASSERT_RULE(lhs.hasShape()); } else { // Otherwise, LHS and RHS must have the same domain. auto lhsDomain = lhs.getRootProtocol(); diff --git a/lib/AST/RequirementMachine/Term.cpp b/lib/AST/RequirementMachine/Term.cpp index 62cd07e6bd201..1f5f902d84f5f 100644 --- a/lib/AST/RequirementMachine/Term.cpp +++ b/lib/AST/RequirementMachine/Term.cpp @@ -68,6 +68,17 @@ Symbol Term::back() const { return Ptr->getElements().back(); } +bool Term::hasShape() const { + return back().getKind() == Symbol::Kind::Shape; +} + +MutableTerm Term::termWithoutShape() const { + if (hasShape()) + return MutableTerm(begin(), end() - 1); + else + return MutableTerm(begin(), end()); +} + Symbol Term::operator[](size_t index) const { return Ptr->getElements()[index]; } @@ -224,6 +235,15 @@ std::optional MutableTerm::compare(const MutableTerm &other, return compareImpl(begin(), end(), other.begin(), other.end(), ctx); } +bool MutableTerm::hasShape() const { + return back().getKind() == Symbol::Kind::Shape; +} + +void MutableTerm::removeShape() { + if (hasShape()) + Symbols.pop_back(); +} + /// Replace the subterm in the range [from,to) of this term with \p rhs. void MutableTerm::rewriteSubTerm(Symbol *from, Symbol *to, Term rhs) { auto oldSize = size(); diff --git a/lib/AST/RequirementMachine/Term.h b/lib/AST/RequirementMachine/Term.h index 8108a6be340fc..9dd01622efea9 100644 --- a/lib/AST/RequirementMachine/Term.h +++ b/lib/AST/RequirementMachine/Term.h @@ -59,6 +59,9 @@ class Term final { Symbol back() const; + bool hasShape() const; + MutableTerm termWithoutShape() const; + Symbol operator[](size_t index) const; /// Returns an opaque pointer that uniquely identifies this term. @@ -184,6 +187,9 @@ class MutableTerm final { return Symbols.back(); } + bool hasShape() const; + void removeShape(); + Symbol operator[](size_t index) const { return Symbols[index]; } From d68a7a7ef0b7e0e1f1ba6b0c899d976500e126a6 Mon Sep 17 00:00:00 2001 From: Kathy Gray Date: Wed, 29 Oct 2025 11:34:06 +0000 Subject: [PATCH 2/2] Requirement Machine: Fix an issue with pack expansion + tuple type crashes Support pack expansion types in term rewriting, maintaining shape invariants and not throwing assertions unnecessarily. Additional tests added for an inifinite case and a concrete case. --- lib/AST/RequirementMachine/InterfaceType.cpp | 12 +++++++++--- test/Generics/infinite_concrete_type.swift | 12 +++++++++++- test/Generics/parameter-pack-requirements.swift | 6 ++++++ ...ewriteContext-getRelativeTermForType-5e95d5.swift | 5 ----- ...ewriteContext-getRelativeTermForType-6235b4.swift | 7 ------- ...ewriteContext-getRelativeTermForType-5e95d5.swift | 4 ++++ ...ewriteContext-getRelativeTermForType-6235b4.swift | 6 ++++++ 7 files changed, 36 insertions(+), 16 deletions(-) delete mode 100644 validation-test/compiler_crashers/RewriteContext-getRelativeTermForType-5e95d5.swift delete mode 100644 validation-test/compiler_crashers/RewriteContext-getRelativeTermForType-6235b4.swift create mode 100644 validation-test/compiler_crashers_fixed/RewriteContext-getRelativeTermForType-5e95d5.swift create mode 100644 validation-test/compiler_crashers_fixed/RewriteContext-getRelativeTermForType-6235b4.swift diff --git a/lib/AST/RequirementMachine/InterfaceType.cpp b/lib/AST/RequirementMachine/InterfaceType.cpp index 600da8a6582ce..44de9cc6c57e3 100644 --- a/lib/AST/RequirementMachine/InterfaceType.cpp +++ b/lib/AST/RequirementMachine/InterfaceType.cpp @@ -424,7 +424,12 @@ RewriteContext::getRelativeTermForType(CanType typeWitness, // Get the substitution S corresponding to τ_0_n. unsigned index = getGenericParamIndex(typeWitness->getRootGenericParam()); result = MutableTerm(substitutions[index]); - ASSERT(!result.hasShape()); + bool hasShape = false; + + if (result.hasShape()) { + hasShape = true; + result.removeShape(); + } // If the substitution is a term consisting of a single protocol symbol // [P], save P for later. @@ -463,6 +468,9 @@ RewriteContext::getRelativeTermForType(CanType typeWitness, for (auto iter = symbols.rbegin(), end = symbols.rend(); iter != end; ++iter) result.add(*iter); + if (hasShape) + result.add(Symbol::forShape(*this)); + return result; } @@ -571,8 +579,6 @@ RewriteContext::getRelativeSubstitutionSchemaFromType( // // τ_0_0 := T // τ_0_1 := U.[shape] - ASSERT(pos != TypePosition::Shape && "Not implemented"); - unsigned index = result.size(); result.push_back(Term::get(term, *this)); diff --git a/test/Generics/infinite_concrete_type.swift b/test/Generics/infinite_concrete_type.swift index 02b271fb02ff1..66a1473a04a79 100644 --- a/test/Generics/infinite_concrete_type.swift +++ b/test/Generics/infinite_concrete_type.swift @@ -55,4 +55,14 @@ protocol TooManyDifferences { associatedtype A2 associatedtype B associatedtype C -} \ No newline at end of file +} + +struct G2 { + func f2() +// expected-error@-1 {{cannot build rewrite system for generic signature; concrete type nesting limit exceeded}} +// expected-note@-2 {{failed rewrite rule is }} +// expected-error@-3 {{generic parameter 'A' is not used in function signature}} + where (repeat each A, T) == T {} +// expected-error@-1 {{tuple with noncopyable element type 'repeat each A' is not supported}} + +} diff --git a/test/Generics/parameter-pack-requirements.swift b/test/Generics/parameter-pack-requirements.swift index fd4b67ed45d35..44a19b4b462f7 100644 --- a/test/Generics/parameter-pack-requirements.swift +++ b/test/Generics/parameter-pack-requirements.swift @@ -126,3 +126,9 @@ func sameTypeMatch1(t: T, u: repeat each U, v: re // CHECK-NEXT: func sameTypeMatch2(t: T, u: repeat each U, v: repeat each V) where T.A == Shape, T.A == Shape {} + +// CHECK-LABEL: PackTupleNesting +// CHECK-NEXT: +struct PackTupleNesting + where T == ((repeat each W), U), + T == (V, (repeat each W)) {} // expected-warning 3{{same-type requirement makes generic parameter}} diff --git a/validation-test/compiler_crashers/RewriteContext-getRelativeTermForType-5e95d5.swift b/validation-test/compiler_crashers/RewriteContext-getRelativeTermForType-5e95d5.swift deleted file mode 100644 index bc8e1c46fd474..0000000000000 --- a/validation-test/compiler_crashers/RewriteContext-getRelativeTermForType-5e95d5.swift +++ /dev/null @@ -1,5 +0,0 @@ -// {"kind":"typecheck","signature":"swift::rewriting::RewriteContext::getRelativeTermForType(swift::CanType, llvm::ArrayRef)","signatureAssert":"Assertion failed: (result.back().getKind() != Symbol::Kind::Shape), function getRelativeTermForType"} -// RUN: not --crash %target-swift-frontend -typecheck %s -extension Result { - func a() where Success == (Result) -> (repeat each b)> {} -} diff --git a/validation-test/compiler_crashers/RewriteContext-getRelativeTermForType-6235b4.swift b/validation-test/compiler_crashers/RewriteContext-getRelativeTermForType-6235b4.swift deleted file mode 100644 index 77622eff8c472..0000000000000 --- a/validation-test/compiler_crashers/RewriteContext-getRelativeTermForType-6235b4.swift +++ /dev/null @@ -1,7 +0,0 @@ -// {"issueID":84490,"kind":"typecheck","signature":"swift::rewriting::RewriteContext::getRelativeTermForType(swift::CanType, llvm::ArrayRef)","signatureAssert":"Assertion failed: (result.back().getKind() != Symbol::Kind::Shape), function getRelativeTermForType"} -// RUN: not --crash %target-swift-frontend -typecheck %s -// https://github.com/swiftlang/swift/issues/84490 -struct a < b > { - func - c < each d where (repeat each d , b) == b>() -} diff --git a/validation-test/compiler_crashers_fixed/RewriteContext-getRelativeTermForType-5e95d5.swift b/validation-test/compiler_crashers_fixed/RewriteContext-getRelativeTermForType-5e95d5.swift new file mode 100644 index 0000000000000..0b0ae3581cbc3 --- /dev/null +++ b/validation-test/compiler_crashers_fixed/RewriteContext-getRelativeTermForType-5e95d5.swift @@ -0,0 +1,4 @@ +// RUN: not %target-swift-frontend -typecheck %s +extension Result { + func a() where Success == (Result) -> (repeat each b)> {} +} diff --git a/validation-test/compiler_crashers_fixed/RewriteContext-getRelativeTermForType-6235b4.swift b/validation-test/compiler_crashers_fixed/RewriteContext-getRelativeTermForType-6235b4.swift new file mode 100644 index 0000000000000..18aaba818889c --- /dev/null +++ b/validation-test/compiler_crashers_fixed/RewriteContext-getRelativeTermForType-6235b4.swift @@ -0,0 +1,6 @@ +// RUN: not %target-swift-frontend -typecheck %s +// https://github.com/swiftlang/swift/issues/84490 +struct a < b > { + func + c < each d where (repeat each d , b) == b>() +}