[RequirementMachine] Avoid passing the requirement error vector through

initialization of the rewrite system.

Instead, the rewrite system can determine trivially redundant requirements
by finding structural requirements with no associated rewrite rules.

Author: hborla

lib/AST/RequirementMachine/ConcreteTypeWitness.cpp

@@ -581,7 +581,7 @@ void PropertyMap::inferConditionalRequirements(
         for (const auto &rule : builder.RequirementRules) {
           auto lhs = std::get<0>(rule);
           auto rhs = std::get<1>(rule);
-          System.addExplicitRule(lhs, rhs, /*requirementID=*/None, errors);
+          System.addExplicitRule(lhs, rhs, /*requirementID=*/None);
         }
       }
     }

lib/AST/RequirementMachine/RequirementMachine.cpp

@@ -75,8 +75,7 @@ void RequirementMachine::checkCompletionResult(CompletionResult result) const {
 ///
 /// Returns failure if completion fails within the configured number of steps.
 std::pair<CompletionResult, unsigned>
-RequirementMachine::initWithGenericSignature(CanGenericSignature sig,
-                                             SmallVectorImpl<RequirementError> &errors) {
+RequirementMachine::initWithGenericSignature(CanGenericSignature sig) {
   Sig = sig;
   Params.append(sig.getGenericParams().begin(),
                 sig.getGenericParams().end());
@@ -99,8 +98,7 @@ RequirementMachine::initWithGenericSignature(CanGenericSignature sig,
                     /*protos=*/ArrayRef<const ProtocolDecl *>(),
                     std::move(builder.WrittenRequirements),
                     std::move(builder.PermanentRules),
-                    std::move(builder.RequirementRules),
-                    errors);
+                    std::move(builder.RequirementRules));
 
   auto result = computeCompletion(RewriteSystem::DisallowInvalidRequirements);
 
@@ -125,8 +123,7 @@ RequirementMachine::initWithGenericSignature(CanGenericSignature sig,
 ///
 /// Returns failure if completion fails within the configured number of steps.
 std::pair<CompletionResult, unsigned>
-RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos,
-                                      SmallVectorImpl<RequirementError> &errors) {
+RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos) {
   FrontendStatsTracer tracer(Stats, "build-rewrite-system");
 
   if (Dump) {
@@ -144,8 +141,7 @@ RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos,
   System.initialize(/*recordLoops=*/true, protos,
                     std::move(builder.WrittenRequirements),
                     std::move(builder.PermanentRules),
-                    std::move(builder.RequirementRules),
-                    errors);
+                    std::move(builder.RequirementRules));
 
   auto result = computeCompletion(RewriteSystem::AllowInvalidRequirements);
 
@@ -171,8 +167,7 @@ RequirementMachine::initWithProtocols(ArrayRef<const ProtocolDecl *> protos,
 std::pair<CompletionResult, unsigned>
 RequirementMachine::initWithWrittenRequirements(
     ArrayRef<GenericTypeParamType *> genericParams,
-    ArrayRef<StructuralRequirement> requirements,
-    SmallVectorImpl<RequirementError> &errors) {
+    ArrayRef<StructuralRequirement> requirements) {
   Params.append(genericParams.begin(), genericParams.end());
 
   FrontendStatsTracer tracer(Stats, "build-rewrite-system");
@@ -194,8 +189,7 @@ RequirementMachine::initWithWrittenRequirements(
                     /*protos=*/ArrayRef<const ProtocolDecl *>(),
                     std::move(builder.WrittenRequirements),
                     std::move(builder.PermanentRules),
-                    std::move(builder.RequirementRules),
-                    errors);
+                    std::move(builder.RequirementRules));
 
   auto result = computeCompletion(RewriteSystem::AllowInvalidRequirements);
 

lib/AST/RequirementMachine/RequirementMachine.h

@@ -91,18 +91,15 @@ class RequirementMachine final {
   void checkCompletionResult(CompletionResult result) const;
 
   std::pair<CompletionResult, unsigned>
-  initWithGenericSignature(CanGenericSignature sig,
-                           SmallVectorImpl<RequirementError> &errors);
+  initWithGenericSignature(CanGenericSignature sig);
 
   std::pair<CompletionResult, unsigned>
-  initWithProtocols(ArrayRef<const ProtocolDecl *> protos,
-                    SmallVectorImpl<RequirementError> &errors);
+  initWithProtocols(ArrayRef<const ProtocolDecl *> protos);
 
   std::pair<CompletionResult, unsigned>
   initWithWrittenRequirements(
       ArrayRef<GenericTypeParamType *> genericParams,
-      ArrayRef<StructuralRequirement> requirements,
-      SmallVectorImpl<RequirementError> &errors);
+      ArrayRef<StructuralRequirement> requirements);
 
   bool isComplete() const;
 

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -354,7 +354,7 @@ RequirementSignatureRequestRQM::evaluate(Evaluator &evaluator,
 
   SmallVector<RequirementError, 4> errors;
 
-  auto status = machine->initWithProtocols(component, errors);
+  auto status = machine->initWithProtocols(component);
   if (status.first != CompletionResult::Success) {
     // All we can do at this point is diagnose and give each protocol an empty
     // requirement signature.
@@ -524,7 +524,7 @@ AbstractGenericSignatureRequestRQM::evaluate(
       ctx.getRewriteContext()));
 
   auto status =
-      machine->initWithWrittenRequirements(genericParams, requirements, errors);
+      machine->initWithWrittenRequirements(genericParams, requirements);
   machine->checkCompletionResult(status.first);
 
   auto minimalRequirements =
@@ -652,7 +652,7 @@ InferredGenericSignatureRequestRQM::evaluate(
       ctx.getRewriteContext()));
 
   auto status =
-      machine->initWithWrittenRequirements(genericParams, requirements, errors);
+      machine->initWithWrittenRequirements(genericParams, requirements);
   if (status.first != CompletionResult::Success) {
     ctx.Diags.diagnose(loc,
                        diag::requirement_machine_completion_failed,

lib/AST/RequirementMachine/RewriteContext.cpp

@@ -133,11 +133,9 @@ RequirementMachine *RewriteContext::getRequirementMachine(
   auto *newMachine = new rewriting::RequirementMachine(*this);
   machine = newMachine;
 
-  SmallVector<RequirementError, 4> errors;
-
   // This might re-entrantly invalidate 'machine', which is a reference
   // into Protos.
-  auto status = newMachine->initWithGenericSignature(sig, errors);
+  auto status = newMachine->initWithGenericSignature(sig);
   newMachine->checkCompletionResult(status.first);
 
   return newMachine;

lib/AST/RequirementMachine/RewriteSystem.cpp

@@ -279,8 +279,7 @@ void RewriteSystem::initialize(
     ArrayRef<StructuralRequirement> writtenRequirements,
     std::vector<std::pair<MutableTerm, MutableTerm>> &&permanentRules,
     std::vector<std::tuple<MutableTerm, MutableTerm, Optional<unsigned>>>
-        &&requirementRules,
-    SmallVectorImpl<RequirementError> &errors) {
+        &&requirementRules) {
   assert(!Initialized);
   Initialized = 1;
 
@@ -295,7 +294,7 @@ void RewriteSystem::initialize(
     auto lhs = std::get<0>(rule);
     auto rhs = std::get<1>(rule);
     auto requirementID = std::get<2>(rule);
-    addExplicitRule(lhs, rhs, requirementID, errors);
+    addExplicitRule(lhs, rhs, requirementID);
   }
 }
 
@@ -496,16 +495,11 @@ bool RewriteSystem::addPermanentRule(MutableTerm lhs, MutableTerm rhs) {
 
 /// Add a new rule, marking it explicit.
 bool RewriteSystem::addExplicitRule(MutableTerm lhs, MutableTerm rhs,
-                                    Optional<unsigned> requirementID,
-                                    SmallVectorImpl<RequirementError> &errors) {
+                                    Optional<unsigned> requirementID) {
   bool added = addRule(std::move(lhs), std::move(rhs));
   if (added) {
     Rules.back().markExplicit();
     Rules.back().setRequirementID(requirementID);
-  } else if (requirementID.hasValue()) {
-    auto req = WrittenRequirements[requirementID.getValue()];
-    errors.push_back(
-        RequirementError::forRedundantRequirement(req.req, req.loc));
   }
 
   return added;
@@ -784,12 +778,23 @@ void RewriteSystem::computeRedundantRequirementDiagnostics(
     return rule.isRedundant();
   };
 
-  for (auto pair : rulesPerRequirement) {
-    unsigned requirementID = pair.first;
-    auto ruleIDs = pair.second;
+  for (auto requirementID : indices(WrittenRequirements)) {
+    auto requirement = WrittenRequirements[requirementID];
+    auto pairIt = rulesPerRequirement.find(requirementID);
+
+    // If there are no rules for this structural requirement, then
+    // the requirement was never added to the rewrite system because
+    // it is trivially redundant.
+    if (pairIt == rulesPerRequirement.end()) {
+      errors.push_back(
+          RequirementError::forRedundantRequirement(requirement.req,
+                                                    requirement.loc));
+      continue;
+    }
 
     // If all rules derived from this structural requirement are redundant,
     // then the requirement is unnecessary in the source code.
+    auto ruleIDs = pairIt->second;
     if (llvm::all_of(ruleIDs, isRedundantRule)) {
       auto requirement = WrittenRequirements[requirementID];
       errors.push_back(

lib/AST/RequirementMachine/RewriteSystem.h

@@ -302,8 +302,7 @@ class RewriteSystem final {
   void initialize(bool recordLoops, ArrayRef<const ProtocolDecl *> protos,
                   ArrayRef<StructuralRequirement> writtenRequirements,
                   std::vector<std::pair<MutableTerm, MutableTerm>> &&permanentRules,
-                  std::vector<std::tuple<MutableTerm, MutableTerm, Optional<unsigned>>> &&requirementRules,
-                  SmallVectorImpl<RequirementError> &errors);
+                  std::vector<std::tuple<MutableTerm, MutableTerm, Optional<unsigned>>> &&requirementRules);
 
   ArrayRef<const ProtocolDecl *> getProtocols() const {
     return Protos;
@@ -336,8 +335,7 @@ class RewriteSystem final {
   bool addPermanentRule(MutableTerm lhs, MutableTerm rhs);
 
   bool addExplicitRule(MutableTerm lhs, MutableTerm rhs,
-                       Optional<unsigned> requirementID,
-                       SmallVectorImpl<RequirementError> &errors);
+                       Optional<unsigned> requirementID);
 
   bool simplify(MutableTerm &term, RewritePath *path=nullptr) const;