[RequirementMachine] Diagnose redundant requirements found during minimization

of the rewrite system.

Author: hborla

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -421,8 +421,16 @@ RequirementSignatureRequestRQM::evaluate(Evaluator &evaluator,
 
   if (ctx.LangOpts.RequirementMachineProtocolSignatures ==
       RequirementMachineMode::Enabled) {
+    // Diagnose trivially invalid requirements from the rewrite
+    // system.
     diagnoseRequirementErrors(ctx, machine->System.getErrors(),
                               /*allowConcreteGenericParams=*/false);
+
+    // Diagnose redundant requirements found during signature
+    // minimization.
+    auto redundancies = machine->System.getRedundantRequirements();
+    diagnoseRequirementErrors(ctx, redundancies,
+                              /*allowConcreteGenericParams=*/false);
   }
 
   // Return the result for the specific protocol this request was kicked off on.
@@ -673,9 +681,20 @@ InferredGenericSignatureRequestRQM::evaluate(
 
   if (ctx.LangOpts.RequirementMachineInferredSignatures ==
       RequirementMachineMode::Enabled) {
-    hadError |= diagnoseRequirementErrors(ctx, errors, allowConcreteGenericParams);
+    // Diagnose invalid requirements dropped during desugaring.
+    hadError |= diagnoseRequirementErrors(ctx, errors,
+                                          allowConcreteGenericParams);
+
+    // Diagnose trivially invalid requirements from the rewrite
+    // system.
     hadError |= diagnoseRequirementErrors(ctx, machine->System.getErrors(),
                                           allowConcreteGenericParams);
+
+    // Diagnose redundant requirements found during signature
+    // minimization.
+    auto redundancies = machine->System.getRedundantRequirements();
+    hadError |= diagnoseRequirementErrors(ctx, redundancies,
+                                          allowConcreteGenericParams);
   }
 
   // FIXME: Handle allowConcreteGenericParams

lib/AST/RequirementMachine/RewriteSystem.cpp

@@ -720,6 +720,50 @@ void RewriteSystem::verifyRewriteRules(ValidityPolicy policy) const {
 #undef ASSERT_RULE
 }
 
+std::vector<RequirementError>
+RewriteSystem::getRedundantRequirements() {
+  // The resulting set of redundant requirements.
+  std::vector<RequirementError> redundantRequirements;
+
+  // Collect all rule IDs for each unique requirement ID.
+  llvm::SmallDenseMap<unsigned, llvm::SmallDenseSet<unsigned, 2>>
+      rulesPerRequirement;
+
+  for (unsigned ruleID : indices(getRules())) {
+    auto &rule = getRules()[ruleID];
+
+    if (!rule.isExplicit())
+      continue;
+
+    auto requirementID = rule.getRequirementID();
+    if (!requirementID.hasValue())
+      continue;
+
+    rulesPerRequirement[*requirementID].insert(ruleID);
+  }
+
+  auto isRedundantRule = [&](unsigned ruleID) {
+    auto &rule = getRules()[ruleID];
+    return rule.isRedundant();
+  };
+
+  for (auto pair : rulesPerRequirement) {
+    unsigned requirementID = pair.first;
+    auto ruleIDs = pair.second;
+
+    // If all rules derived from this structural requirement are redundant,
+    // then the requirement is unnecessary in the source code.
+    if (llvm::all_of(ruleIDs, isRedundantRule)) {
+      auto requirement = WrittenRequirements[requirementID];
+      redundantRequirements.push_back(
+          RequirementError::forRedundantRequirement(requirement.req,
+                                                    requirement.loc));
+    }
+  }
+
+  return redundantRequirements;
+}
+
 void RewriteSystem::dump(llvm::raw_ostream &out) const {
   out << "Rewrite system: {\n";
   for (const auto &rule : Rules) {

lib/AST/RequirementMachine/RewriteSystem.h

@@ -296,10 +296,6 @@ class RewriteSystem final {
     return ProtocolMap;
   }
 
-  ArrayRef<RequirementError> getErrors() {
-    return Errors;
-  }
-
   DebugOptions getDebugOptions() const { return Debug; }
 
   void initialize(bool recordLoops, ArrayRef<const ProtocolDecl *> protos,
@@ -372,6 +368,18 @@ class RewriteSystem final {
 
   void verifyRewriteRules(ValidityPolicy policy) const;
 
+  //////////////////////////////////////////////////////////////////////////////
+  ///
+  /// Diagnostics
+  ///
+  //////////////////////////////////////////////////////////////////////////////
+
+  ArrayRef<RequirementError> getErrors() {
+    return Errors;
+  }
+
+  std::vector<RequirementError> getRedundantRequirements();
+
 private:
   struct CriticalPair {
     MutableTerm LHS;

test/Generics/requirement_machine_diagnostics.swift

@@ -111,3 +111,49 @@ protocol TrivialRedundantSameType where Self == Self {
   associatedtype T where T == T
   // expected-warning@-1 {{redundant same-type constraint 'Self.T' == 'Self.T'}}
 }
+
+struct G<T> { }
+
+protocol Pair {
+  associatedtype A
+  associatedtype B
+}
+
+func test1<T: Pair>(_: T) where T.A == G<Int>, T.A == G<T.B>, T.B == Int { }
+// expected-warning@-1 {{redundant same-type constraint 'T.A' == 'G<T.B>'}}
+
+
+protocol P1 {
+  func p1()
+}
+
+protocol P2 : P1 { }
+
+protocol P3 {
+  associatedtype P3Assoc : P2
+}
+
+protocol P4 {
+  associatedtype P4Assoc : P1
+}
+
+func inferSameType2<T : P3, U : P4>(_: T, _: U) where U.P4Assoc : P2, T.P3Assoc == U.P4Assoc {}
+// expected-warning@-1{{redundant conformance constraint 'U.P4Assoc' : 'P2'}}
+
+protocol P5 {
+  associatedtype Element
+}
+
+protocol P6 {
+  associatedtype AssocP6 : P5
+}
+
+protocol P7 : P6 {
+  associatedtype AssocP7: P6
+}
+
+extension P7 where AssocP6.Element : P6,
+        AssocP7.AssocP6.Element : P6, // expected-warning{{redundant conformance constraint 'Self.AssocP7.AssocP6.Element' : 'P6'}}
+        AssocP6.Element == AssocP7.AssocP6.Element {
+  func nestedSameType1() { }
+}