Merge pull request #84675 from slavapestov/rqm-protocol-failure-bookkeeping

slavapestov · web-flow · commit 811a201bc13d · 2025-10-05T14:10:00.000-04:00
RequirementMachine: New way of propagating failure when building rewrite system for protocol
diff --git 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<GenericSignatureErrorFlags>;
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -7451,7 +7451,7 @@ RequirementSignature ProtocolDecl::getRequirementSignature() const {
                RequirementSignatureRequest{const_cast<ProtocolDecl *>(this)},
                [this]() {
                  return RequirementSignature::getPlaceholderRequirementSignature(
-                     this, GenericSignatureErrors());
+                     this, GenericSignatureErrorFlags::CircularReference);
                });
 }
 
diff --git 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<ProtocolDecl *, 2> inheritedProtos;
-  for (auto *inheritedProto : proto->getInheritedProtocols()) {
-    inheritedProtos.push_back(inheritedProto);
-  }
-
+  // Pretend the protocol inherits from Copyable and Escapable.
+  SmallVector<Requirement, 2> requirements;
   for (auto ip : InvertibleProtocolSet::allKnown()) {
     auto *otherProto = ctx.getProtocol(getKnownProtocolKind(ip));
-    inheritedProtos.push_back(otherProto);
-  }
-
-  ProtocolType::canonicalizeProtocols(inheritedProtos);
-
-  SmallVector<Requirement, 2> 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/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<ArchetypeType>())
-    return archetypeType->getNestedType(assocType);
+  if (auto *archetypeType = conformingType->getAs<ArchetypeType>()) {
+    auto witnessType = archetypeType->getNestedType(assocType);
+    if (!witnessType)
+      return ErrorType::get(assocType->getASTContext());
+    return witnessType;
+  }
 
   return DependentMemberType::get(conformingType, assocType);
 }
diff --git 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<GenericTypeParamType *> 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
@@ -395,6 +395,9 @@ RequirementMachine::buildRequirementsFromRules(
     bool reconstituteSugar,
     std::vector<Requirement> &reqs,
     std::vector<ProtocolTypeAlias> &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
@@ -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<const ProtocolDecl *>(),
@@ -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<const ProtocolDecl *>(),
@@ -553,10 +565,6 @@ ArrayRef<Rule> 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
@@ -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<GenericTypeParamType *> genericParams,
       ArrayRef<StructuralRequirement> requirements);
 
-  bool isComplete() const;
+  bool isComplete() const {
+    return Complete;
+  }
 
   std::pair<CompletionResult, unsigned>
   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
@@ -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
@@ -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<const ProtocolDecl *> &referencedProtocols)
       : Context(ctx), ReferencedProtocols(referencedProtocols) {
     Dump = ctx.getASTContext().LangOpts.DumpRequirementMachine;
     Initialized = 0;
+    Failed = 0;
   }
 
   void initWithGenericSignature(ArrayRef<GenericTypeParamType *> genericParams,
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -5992,7 +5992,7 @@ NeverNullType TypeResolver::resolveTupleType(TupleTypeRepr *repr,
         !isa<TupleTypeRepr>(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
@@ -23,3 +23,7 @@ func inferredTuples<T>(x: Int, y: borrowing Butt, z: T) {
     _ = b
     _ = c
 }
+
+// Avoid spurious diagnostic with the tuple here
+func bogus<T>(_: T, _: (T, T)) where T == DoesNotExist {}
+// expected-error@-1 {{cannot find type 'DoesNotExist' in scope}}
diff --git 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 : P1 & P2>(_: 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
@@ -7,12 +7,16 @@ protocol MyCollectionProtocol: Collection where Iterator == MyCollectionIterator
 
 struct MyCollectionIterator<MyCollection: MyCollectionProtocol>: IteratorProtocol {
 // expected-note@-1 3{{through reference here}}
+// expected-error@-2 {{type 'MyCollectionIterator<MyCollection>' 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_fixed/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<Self> {
   struct b<c: a>: IteratorProtocol
     func d ->
diff --git a/validation-test/compiler_crashers_2_fixed/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<c
   extension b: BidirectionalCollection where c: a
diff --git a/validation-test/compiler_crashers_2_fixed/3badfb288fdf42f4.swift b/validation-test/compiler_crashers_2_fixed/3badfb288fdf42f4.swift
@@ -1,4 +1,4 @@
 // {"kind":"typecheck","signature":"(anonymous namespace)::TypeSubstituter::transformDependentMemberType(swift::DependentMemberType*, swift::TypePosition)","signatureAssert":"Assertion failed: (Ptr && \"Cannot dereference a null Type!\"), function operator->"}
-// 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 <b , c > where Upstream == Publishers.FlatMap <b, c>
diff --git a/validation-test/compiler_crashers_2_fixed/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

Original file line number	Diff line number	Diff line change
`@@ -7451,7 +7451,7 @@ RequirementSignature ProtocolDecl::getRequirementSignature() const {`
`7451`	`7451`	`RequirementSignatureRequest{const_cast<ProtocolDecl *>(this)},`
`7452`	`7452`	`[this]() {`
`7453`	`7453`	`return RequirementSignature::getPlaceholderRequirementSignature(`
`7454`		`- this, GenericSignatureErrors());`
	`7454`	`+ this, GenericSignatureErrorFlags::CircularReference);`
`7455`	`7455`	`});`
`7456`	`7456`	`}`
`7457`	`7457`