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

RequirementMachine: New way of propagating failure when building rewrite system for protocol

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

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,