From 1731b09a88cb4a95e4d022ad32f4480c8841dba0 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 3 Oct 2025 14:01:57 -0400 Subject: [PATCH 1/3] AST: Don't return null Type() from getTypeWitness() --- lib/AST/ProtocolConformanceRef.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/AST/ProtocolConformanceRef.cpp b/lib/AST/ProtocolConformanceRef.cpp index d445040f7c2f1..0a1ecfa471b25 100644 --- a/lib/AST/ProtocolConformanceRef.cpp +++ b/lib/AST/ProtocolConformanceRef.cpp @@ -202,8 +202,12 @@ Type ProtocolConformanceRef::getTypeWitness(AssociatedTypeDecl *assocType, auto conformingType = abstract->getType(); ASSERT(abstract->getProtocol() == assocType->getProtocol()); - if (auto *archetypeType = conformingType->getAs()) - return archetypeType->getNestedType(assocType); + if (auto *archetypeType = conformingType->getAs()) { + auto witnessType = archetypeType->getNestedType(assocType); + if (!witnessType) + return ErrorType::get(assocType->getASTContext()); + return witnessType; + } return DependentMemberType::get(conformingType, assocType); } From 21214bea3ca160fe51f1c9bd943b7afc917c7556 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 3 Oct 2025 14:02:48 -0400 Subject: [PATCH 2/3] Sema: Fix spurious diagnostic about move-only tuples --- lib/Sema/TypeCheckType.cpp | 2 +- test/Constraints/moveonly_tuples.swift | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp index b49c5ba34a9cb..680de34d68218 100644 --- a/lib/Sema/TypeCheckType.cpp +++ b/lib/Sema/TypeCheckType.cpp @@ -5992,7 +5992,7 @@ NeverNullType TypeResolver::resolveTupleType(TupleTypeRepr *repr, !isa(tyR)) { auto contextTy = GenericEnvironment::mapTypeIntoContext( resolution.getGenericSignature().getGenericEnvironment(), ty); - if (contextTy->isNoncopyable()) + if (!contextTy->hasError() && contextTy->isNoncopyable()) moveOnlyElementIndex = i; } diff --git a/test/Constraints/moveonly_tuples.swift b/test/Constraints/moveonly_tuples.swift index 28ed100bfe389..5fed385a2b512 100644 --- a/test/Constraints/moveonly_tuples.swift +++ b/test/Constraints/moveonly_tuples.swift @@ -23,3 +23,7 @@ func inferredTuples(x: Int, y: borrowing Butt, z: T) { _ = b _ = c } + +// Avoid spurious diagnostic with the tuple here +func bogus(_: T, _: (T, T)) where T == DoesNotExist {} +// expected-error@-1 {{cannot find type 'DoesNotExist' in scope}} From 244d2afea35f0b313dabb2224c55877ebc321182 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 3 Oct 2025 14:07:53 -0400 Subject: [PATCH 3/3] RequirementMachine: New way of propagating failure when building rewrite system for protocol If we failed to construct a rewrite system for a protocol, either because the Knuth-Bendix algorithm failed or because of a request cycle while resolving requirements, we would end up in a situation where the resulting rewrite system didn't include all conformance requirements and associated types, so name lookup would find declarations whose interface types are not valid type parameters. Fix this by propagating failure better and just doing nothing in getReducedTypeParameter(). Fixes rdar://147277543. --- include/swift/AST/GenericSignature.h | 6 ++++- lib/AST/Decl.cpp | 2 +- lib/AST/GenericSignature.cpp | 26 +++---------------- .../GenericSignatureQueries.cpp | 6 +++++ .../RequirementMachine/RequirementBuilder.cpp | 3 +++ .../RequirementMachine/RequirementMachine.cpp | 16 +++++++++--- .../RequirementMachine/RequirementMachine.h | 15 ++++++++++- lib/AST/RequirementMachine/RuleBuilder.cpp | 20 +++++--------- lib/AST/RequirementMachine/RuleBuilder.h | 4 +++ test/Generics/non_fcrs.swift | 4 +++ test/Generics/rdar94848868.swift | 4 +++ .../15d1c6f76ad86540.swift | 2 +- .../03ac172bc4a98f31.swift | 2 +- .../3badfb288fdf42f4.swift | 2 +- .../8a6431938ab864b3.swift | 2 +- 15 files changed, 67 insertions(+), 47 deletions(-) rename validation-test/IDE/{crashers => crashers_fixed}/15d1c6f76ad86540.swift (65%) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/03ac172bc4a98f31.swift (81%) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/3badfb288fdf42f4.swift (87%) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/8a6431938ab864b3.swift (84%) diff --git a/include/swift/AST/GenericSignature.h b/include/swift/AST/GenericSignature.h index e04ef58f1f12b..d4fd6dc357baa 100644 --- a/include/swift/AST/GenericSignature.h +++ b/include/swift/AST/GenericSignature.h @@ -606,7 +606,11 @@ enum class GenericSignatureErrorFlags { /// The Knuth-Bendix completion procedure failed to construct a confluent /// rewrite system. - CompletionFailed = (1<<2) + CompletionFailed = (1<<2), + + /// A request evaluator cycle prevented us from computing this generic + /// signature. + CircularReference = (1<<3), }; using GenericSignatureErrors = OptionSet; diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index a9adbe8406313..ea76f64a38cb6 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -7451,7 +7451,7 @@ RequirementSignature ProtocolDecl::getRequirementSignature() const { RequirementSignatureRequest{const_cast(this)}, [this]() { return RequirementSignature::getPlaceholderRequirementSignature( - this, GenericSignatureErrors()); + this, GenericSignatureErrorFlags::CircularReference); }); } diff --git a/lib/AST/GenericSignature.cpp b/lib/AST/GenericSignature.cpp index fd86d8ac68f98..07d87aa04b8fc 100644 --- a/lib/AST/GenericSignature.cpp +++ b/lib/AST/GenericSignature.cpp @@ -1400,33 +1400,13 @@ RequirementSignature RequirementSignature::getPlaceholderRequirementSignature( const ProtocolDecl *proto, GenericSignatureErrors errors) { auto &ctx = proto->getASTContext(); - SmallVector inheritedProtos; - for (auto *inheritedProto : proto->getInheritedProtocols()) { - inheritedProtos.push_back(inheritedProto); - } - + // Pretend the protocol inherits from Copyable and Escapable. + SmallVector requirements; for (auto ip : InvertibleProtocolSet::allKnown()) { auto *otherProto = ctx.getProtocol(getKnownProtocolKind(ip)); - inheritedProtos.push_back(otherProto); - } - - ProtocolType::canonicalizeProtocols(inheritedProtos); - - SmallVector requirements; - - for (auto *inheritedProto : inheritedProtos) { requirements.emplace_back(RequirementKind::Conformance, proto->getSelfInterfaceType(), - inheritedProto->getDeclaredInterfaceType()); - } - - for (auto *assocTypeDecl : proto->getAssociatedTypeMembers()) { - for (auto ip : InvertibleProtocolSet::allKnown()) { - auto *otherProto = ctx.getProtocol(getKnownProtocolKind(ip)); - requirements.emplace_back(RequirementKind::Conformance, - assocTypeDecl->getDeclaredInterfaceType(), - otherProto->getDeclaredInterfaceType()); - } + otherProto->getDeclaredInterfaceType()); } // Maintain invariants. diff --git a/lib/AST/RequirementMachine/GenericSignatureQueries.cpp b/lib/AST/RequirementMachine/GenericSignatureQueries.cpp index 3d8648ee72397..66d18b5fce70c 100644 --- a/lib/AST/RequirementMachine/GenericSignatureQueries.cpp +++ b/lib/AST/RequirementMachine/GenericSignatureQueries.cpp @@ -353,6 +353,9 @@ static Type substPrefixType(Type type, unsigned suffixLength, Type prefixType, Type RequirementMachine::getReducedTypeParameter( CanType t, ArrayRef genericParams) const { + if (Failed) + return ErrorType::get(t); + // Get a simplified term T. auto term = Context.getMutableTermForType(t, /*proto=*/nullptr); System.simplify(term); @@ -801,6 +804,9 @@ void RequirementMachine::verify(const MutableTerm &term) const { } } + if (Failed) + return; + MutableTerm erased; // First, "erase" resolved associated types from the term, and try diff --git a/lib/AST/RequirementMachine/RequirementBuilder.cpp b/lib/AST/RequirementMachine/RequirementBuilder.cpp index a0bfcccbb7761..21da0433bbf4d 100644 --- a/lib/AST/RequirementMachine/RequirementBuilder.cpp +++ b/lib/AST/RequirementMachine/RequirementBuilder.cpp @@ -395,6 +395,9 @@ RequirementMachine::buildRequirementsFromRules( bool reconstituteSugar, std::vector &reqs, std::vector &aliases) const { + if (Failed) + return; + RequirementBuilder builder(System, Map, genericParams, reconstituteSugar); builder.addRequirementRules(requirementRules); diff --git a/lib/AST/RequirementMachine/RequirementMachine.cpp b/lib/AST/RequirementMachine/RequirementMachine.cpp index c4dd539b3bf6a..c5a37777a87c0 100644 --- a/lib/AST/RequirementMachine/RequirementMachine.cpp +++ b/lib/AST/RequirementMachine/RequirementMachine.cpp @@ -289,6 +289,9 @@ RequirementMachine::initWithProtocolSignatureRequirements( RuleBuilder builder(Context, System.getReferencedProtocols()); builder.initWithProtocolSignatureRequirements(protos); + // Remember if any of our upstream protocols failed to complete. + Failed = builder.Failed; + // Add the initial set of rewrite rules to the rewrite system. System.initialize(/*recordLoops=*/false, protos, std::move(builder.ImportedRules), @@ -337,6 +340,9 @@ RequirementMachine::initWithGenericSignature(GenericSignature sig) { builder.initWithGenericSignature(sig.getGenericParams(), sig.getRequirements()); + // Remember if any of our upstream protocols failed to complete. + Failed = builder.Failed; + // Add the initial set of rewrite rules to the rewrite system. System.initialize(/*recordLoops=*/false, /*protos=*/ArrayRef(), @@ -390,6 +396,9 @@ RequirementMachine::initWithProtocolWrittenRequirements( RuleBuilder builder(Context, System.getReferencedProtocols()); builder.initWithProtocolWrittenRequirements(component, protos); + // Remember if any of our upstream protocols failed to complete. + Failed = builder.Failed; + // Add the initial set of rewrite rules to the rewrite system. System.initialize(/*recordLoops=*/true, component, std::move(builder.ImportedRules), @@ -437,6 +446,9 @@ RequirementMachine::initWithWrittenRequirements( RuleBuilder builder(Context, System.getReferencedProtocols()); builder.initWithWrittenRequirements(genericParams, requirements); + // Remember if any of our upstream protocols failed to complete. + Failed = builder.Failed; + // Add the initial set of rewrite rules to the rewrite system. System.initialize(/*recordLoops=*/true, /*protos=*/ArrayRef(), @@ -553,10 +565,6 @@ ArrayRef RequirementMachine::getLocalRules() const { return System.getLocalRules(); } -bool RequirementMachine::isComplete() const { - return Complete; -} - GenericSignatureErrors RequirementMachine::getErrors() const { // FIXME: Assert if we had errors but we didn't emit any diagnostics? return System.getErrors(); diff --git a/lib/AST/RequirementMachine/RequirementMachine.h b/lib/AST/RequirementMachine/RequirementMachine.h index 91f3b862ce23d..1b1592bd5c51b 100644 --- a/lib/AST/RequirementMachine/RequirementMachine.h +++ b/lib/AST/RequirementMachine/RequirementMachine.h @@ -64,6 +64,13 @@ class RequirementMachine final { bool Dump = false; bool Complete = false; + /// Whether completion failed, either for this rewrite system, or one of + /// its imported protocols. In this case, name lookup might find type + /// parameters that are not valid according to the rewrite system, because + /// not all conformance requirements will be present. This flag allows + /// us to skip certain verification checks in that case. + bool Failed = false; + /// Parameters to prevent runaway completion and property map construction. unsigned MaxRuleCount; unsigned MaxRuleLength; @@ -110,7 +117,9 @@ class RequirementMachine final { ArrayRef genericParams, ArrayRef requirements); - bool isComplete() const; + bool isComplete() const { + return Complete; + } std::pair computeCompletion(RewriteSystem::ValidityPolicy policy); @@ -139,6 +148,10 @@ class RequirementMachine final { public: ~RequirementMachine(); + bool isFailed() const { + return Failed; + } + // Generic signature queries. Generally you shouldn't have to construct a // RequirementMachine instance; instead, call the corresponding methods on // GenericSignature, which lazily create a RequirementMachine for you. diff --git a/lib/AST/RequirementMachine/RuleBuilder.cpp b/lib/AST/RequirementMachine/RuleBuilder.cpp index 499480cd0f580..3ba68101fa0c3 100644 --- a/lib/AST/RequirementMachine/RuleBuilder.cpp +++ b/lib/AST/RequirementMachine/RuleBuilder.cpp @@ -106,19 +106,10 @@ void RuleBuilder::initWithProtocolSignatureRequirements( addPermanentProtocolRules(proto); auto reqs = proto->getRequirementSignature(); - - // If completion failed, we'll have a totally empty requirement signature, - // but to maintain invariants around what constitutes a valid rewrite term - // between getTypeForTerm() and isValidTypeParameter(), we need to add rules - // for inherited protocols. - if (reqs.getErrors().contains(GenericSignatureErrorFlags::CompletionFailed)) { - for (auto *inheritedProto : proto->getAllInheritedProtocols()) { - Requirement req(RequirementKind::Conformance, - proto->getSelfInterfaceType(), - inheritedProto->getDeclaredInterfaceType()); - - addRequirement(req.getCanonical(), proto); - } + auto errors = reqs.getErrors(); + if (errors.contains(GenericSignatureErrorFlags::CompletionFailed) || + errors.contains(GenericSignatureErrorFlags::CircularReference)) { + Failed = 1; } for (auto req : reqs.getRequirements()) @@ -495,6 +486,9 @@ void RuleBuilder::collectRulesFromReferencedProtocols() { continue; } + if (machine->isFailed()) + Failed = 1; + // We grab the machine's local rules, not *all* of its rules, to avoid // duplicates in case multiple machines share a dependency on a downstream // protocol component. diff --git a/lib/AST/RequirementMachine/RuleBuilder.h b/lib/AST/RequirementMachine/RuleBuilder.h index e1ce2e095101c..cc96f36810fb7 100644 --- a/lib/AST/RequirementMachine/RuleBuilder.h +++ b/lib/AST/RequirementMachine/RuleBuilder.h @@ -83,11 +83,15 @@ struct RuleBuilder { /// Used to ensure the initWith*() methods are only called once. unsigned Initialized : 1; + /// Whether completion failed for any of our upstream protocol components. + unsigned Failed : 1; + RuleBuilder(RewriteContext &ctx, llvm::DenseSet &referencedProtocols) : Context(ctx), ReferencedProtocols(referencedProtocols) { Dump = ctx.getASTContext().LangOpts.DumpRequirementMachine; Initialized = 0; + Failed = 0; } void initWithGenericSignature(ArrayRef genericParams, diff --git a/test/Generics/non_fcrs.swift b/test/Generics/non_fcrs.swift index 19767c1f2e9b9..84966b4d8e21e 100644 --- a/test/Generics/non_fcrs.swift +++ b/test/Generics/non_fcrs.swift @@ -31,6 +31,10 @@ protocol P2 { associatedtype T : P2 } +protocol P12: P1, P2 {} +// expected-error@-1 {{cannot build rewrite system for protocol; rule length limit exceeded}} +// expected-note@-2 {{failed rewrite rule is [P12:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P2] => [P12:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T]}} + func foo(_: T) {} // expected-error@-1 {{cannot build rewrite system for generic signature; rule length limit exceeded}} // expected-note@-2 {{failed rewrite rule is τ_0_0.[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P2] => τ_0_0.[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T].[P1:T]}} diff --git a/test/Generics/rdar94848868.swift b/test/Generics/rdar94848868.swift index ae4ceadca9029..cab377a9a81c1 100644 --- a/test/Generics/rdar94848868.swift +++ b/test/Generics/rdar94848868.swift @@ -7,12 +7,16 @@ protocol MyCollectionProtocol: Collection where Iterator == MyCollectionIterator struct MyCollectionIterator: IteratorProtocol { // expected-note@-1 3{{through reference here}} +// expected-error@-2 {{type 'MyCollectionIterator' does not conform to protocol 'IteratorProtocol'}} +// expected-note@-3 {{add stubs for conformance}} mutating func next() -> MyCollection.Element? { + // expected-error@-1 {{'Element' is not a member type of type 'MyCollection'}} return nil } } struct MyCollection: MyCollectionProtocol { +// expected-error@-1 {{type 'MyCollection' does not conform to protocol 'Collection'}} struct Element {} var startIndex: Int { fatalError() } diff --git a/validation-test/IDE/crashers/15d1c6f76ad86540.swift b/validation-test/IDE/crashers_fixed/15d1c6f76ad86540.swift similarity index 65% rename from validation-test/IDE/crashers/15d1c6f76ad86540.swift rename to validation-test/IDE/crashers_fixed/15d1c6f76ad86540.swift index cd53a554f91f8..e51ce075c29dc 100644 --- a/validation-test/IDE/crashers/15d1c6f76ad86540.swift +++ b/validation-test/IDE/crashers_fixed/15d1c6f76ad86540.swift @@ -1,5 +1,5 @@ // {"kind":"complete","signature":"swift::GenericSignatureImpl::getReducedType(swift::Type) const"} -// RUN: not --crash %target-swift-ide-test -code-completion --code-completion-token=COMPLETE -source-filename %s +// RUN: %target-swift-ide-test -code-completion --code-completion-token=COMPLETE -source-filename %s protocol a: Collection where Iterator == b { struct b: IteratorProtocol func d -> diff --git a/validation-test/compiler_crashers_2/03ac172bc4a98f31.swift b/validation-test/compiler_crashers_2_fixed/03ac172bc4a98f31.swift similarity index 81% rename from validation-test/compiler_crashers_2/03ac172bc4a98f31.swift rename to validation-test/compiler_crashers_2_fixed/03ac172bc4a98f31.swift index e04a6d587319c..30b29c034dbe0 100644 --- a/validation-test/compiler_crashers_2/03ac172bc4a98f31.swift +++ b/validation-test/compiler_crashers_2_fixed/03ac172bc4a98f31.swift @@ -1,5 +1,5 @@ // {"kind":"typecheck","original":"73696db0","signature":"swift::GenericSignatureImpl::isReducedType(swift::Type) const"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s a struct b"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s // REQUIRES: OS=macosx import Combine extension Publishers.Share where Upstream == {a where Upstream == Publishers.FlatMap diff --git a/validation-test/compiler_crashers_2/8a6431938ab864b3.swift b/validation-test/compiler_crashers_2_fixed/8a6431938ab864b3.swift similarity index 84% rename from validation-test/compiler_crashers_2/8a6431938ab864b3.swift rename to validation-test/compiler_crashers_2_fixed/8a6431938ab864b3.swift index f851a2467b981..e6e63fd903058 100644 --- a/validation-test/compiler_crashers_2/8a6431938ab864b3.swift +++ b/validation-test/compiler_crashers_2_fixed/8a6431938ab864b3.swift @@ -1,5 +1,5 @@ // {"signature":"swift::rewriting::RequirementMachine::checkCompletionResult(swift::rewriting::CompletionResult) const"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s protocol a : b protocol b{associatedtype c} protocol d : e, f protocol e : g where c : e protocol g : b protocol f : a where c : f