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.

Author: slavapestov

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>;

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);
                });
 }
 

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.

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

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);

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();

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.

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.

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,

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]}}