[RequirementMachine] Simplify computing and emitting conflict diagnostics.

hborla · hborla · commit a4230a60616f · 2022-03-29T18:27:45.000-07:00
Instead of computing different combinations of conflicting requirement kinds,
emit the same error message for all of them.
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -2509,6 +2509,9 @@ WARNING(missing_protocol_refinement, none,
 ERROR(same_type_conflict,none,
       "%select{generic parameter |protocol |}0%1 cannot be equal to both "
       "%2 and %3", (unsigned, Type, Type, Type))
+ERROR(requirement_conflict,none,
+      "no type for %0 can satisfy both %1",
+      (Type, StringRef))
 WARNING(redundant_same_type_to_concrete,none,
         "redundant same-type constraint %0 == %1", (Type, Type))
 NOTE(same_type_redundancy_here,none,
diff --git a/lib/AST/RequirementMachine/Diagnostics.h b/lib/AST/RequirementMachine/Diagnostics.h
@@ -39,26 +39,23 @@ struct RequirementError {
     RedundantRequirement,
   } kind;
 
-  /// The type parameter on which there is an invalid or conflicting
-  /// requirement.
-  ///
-  /// FIXME: We probably want to just store two separate requirements
-  /// in the case of a confict. Right now, the conflicting constraint
-  /// types are both stored in the requirement below, and this serves
-  /// as the subject type.
-  Type typeParameter;
-
   /// The invalid requirement.
   Requirement requirement;
 
+  /// A requirement that conflicts with \c requirement. Both
+  /// requirements will have the same subject type.
+  Optional<Requirement> conflictingRequirement;
+
   SourceLoc loc;
 
 private:
   RequirementError(Kind kind, Requirement requirement, SourceLoc loc)
-    : kind(kind), typeParameter(Type()), requirement(requirement), loc(loc) {}
+    : kind(kind), requirement(requirement), conflictingRequirement(None), loc(loc) {}
 
-  RequirementError(Kind kind, Type subject, Requirement requirement, SourceLoc loc)
-    : kind(kind), typeParameter(subject), requirement(requirement), loc(loc) {}
+  RequirementError(Kind kind, Requirement requirement,
+                   Requirement conflict,
+                   SourceLoc loc)
+    : kind(kind), requirement(requirement), conflictingRequirement(conflict), loc(loc) {}
 
 public:
   static RequirementError forInvalidTypeRequirement(Type subjectType,
@@ -78,10 +75,10 @@ struct RequirementError {
     return {Kind::ConflictingRequirement, req, loc};
   }
 
-  static RequirementError forConflictingRequirement(Type typeParameter,
-                                                    Requirement req,
+  static RequirementError forConflictingRequirement(Requirement first,
+                                                    Requirement second,
                                                     SourceLoc loc) {
-    return {Kind::ConflictingRequirement, typeParameter, req, loc};
+    return {Kind::ConflictingRequirement, first, second, loc};
   }
 
   static RequirementError forRedundantRequirement(Requirement req,
diff --git a/lib/AST/RequirementMachine/RequirementLowering.cpp b/lib/AST/RequirementMachine/RequirementLowering.cpp
@@ -723,44 +723,29 @@ bool swift::rewriting::diagnoseRequirementErrors(
 
     case RequirementError::Kind::ConflictingRequirement: {
       auto requirement = error.requirement;
-      auto subjectType = error.typeParameter;
-      switch (requirement.getKind()) {
-      case RequirementKind::SameType:
+      auto conflict = error.conflictingRequirement;
+
+      if (!conflict) {
         if (requirement.getFirstType()->hasError() ||
             requirement.getSecondType()->hasError()) {
           // Don't emit a cascading error.
-        } else if (subjectType) {
-          ctx.Diags.diagnose(loc, diag::same_type_conflict,
-                             false, subjectType,
-                             requirement.getFirstType(),
-                             requirement.getSecondType());
-        } else {
-          ctx.Diags.diagnose(loc, diag::requires_same_concrete_type,
-                             requirement.getFirstType(),
-                             requirement.getSecondType());
+          break;
         }
-        break;
-      case RequirementKind::Conformance:
-        ctx.Diags.diagnose(loc, diag::requires_generic_param_same_type_does_not_conform,
+        ctx.Diags.diagnose(loc, diag::requires_same_concrete_type,
                            requirement.getFirstType(),
-                           requirement.getProtocolDecl()->getName());
-        break;
-      case RequirementKind::Superclass:
-        if (subjectType) {
-          ctx.Diags.diagnose(loc, diag::conflicting_superclass_constraints,
-                             subjectType, requirement.getFirstType(),
-                             requirement.getSecondType());
-        } else {
-          ctx.Diags.diagnose(loc, diag::same_type_does_not_inherit,
-                             requirement.getFirstType(),
-                             requirement.getSecondType());
-        }
-        break;
-      case RequirementKind::Layout:
-        ctx.Diags.diagnose(loc, diag::requires_generic_param_same_type_does_not_conform,
-                           requirement.getFirstType(),
-                           ctx.getIdentifier(requirement.getLayoutConstraint()->getName()));
-        break;
+                           requirement.getSecondType());
+      } else {
+        auto options = PrintOptions::forDiagnosticArguments();
+        std::string requirements;
+        llvm::raw_string_ostream OS(requirements);
+        OS << "'";
+        requirement.print(OS, options);
+        OS << "' and '";
+        conflict->print(OS, options);
+        OS << "'";
+
+        ctx.Diags.diagnose(loc, diag::requirement_conflict,
+                           requirement.getFirstType(), requirements);
       }
 
       diagnosedError = true;
diff --git a/lib/AST/RequirementMachine/RewriteSystem.cpp b/lib/AST/RequirementMachine/RewriteSystem.cpp
@@ -776,80 +776,10 @@ void RewriteSystem::computeConflictDiagnostics(
       continue;
 
     Type subject = propertyMap.getTypeForTerm(subjectTerm, genericParams);
-
-    // Record conflicting requirements on a type parameter, e.g.
-    // conflicting superclass requirements:
-    //
-    //   class C1 {}
-    //   class C2 {}
-    //   protocol P { associatedtype A: C1 }
-    //   func conflict<T: P>(_: T) where T.A: C2 {}
-    if (firstProperty->getKind() == secondProperty->getKind() &&
-        firstTerm.back().getKind() != Symbol::Kind::Name) {
-      switch (firstProperty->getKind()) {
-      case Symbol::Kind::ConcreteType:
-        errors.push_back(RequirementError::forConflictingRequirement(subject,
-            {RequirementKind::SameType, firstProperty->getConcreteType(),
-             secondProperty->getConcreteType()},
-            signatureLoc));
-        continue;
-
-      case Symbol::Kind::Superclass:
-        // FIXME: shoving the conflicting superclass types into a superclass
-        // requiement is a little gross.
-        errors.push_back(RequirementError::forConflictingRequirement(subject,
-            {RequirementKind::Superclass, firstProperty->getConcreteType(),
-             secondProperty->getConcreteType()},
-            signatureLoc));
-        continue;
-
-      // FIXME: Conflicting layout requirements?
-      default:
-        continue;
-      }
-    }
-
-    auto recordError = [&](Symbol subject, Symbol constraint) {
-      auto subjectType = subject.getConcreteType();
-      switch (constraint.getKind()) {
-      case Symbol::Kind::ConcreteType:
-        errors.push_back(RequirementError::forConflictingRequirement(
-            {RequirementKind::SameType, subjectType, constraint.getConcreteType()},
-            signatureLoc));
-        return;
-
-      case Symbol::Kind::Superclass:
-        errors.push_back(RequirementError::forConflictingRequirement(
-            {RequirementKind::Superclass, subjectType, constraint.getConcreteType()},
-            signatureLoc));
-        return;
-
-      case Symbol::Kind::Protocol:
-        errors.push_back(RequirementError::forConflictingRequirement(
-            {RequirementKind::Conformance, subjectType,
-             constraint.getProtocol()->getDeclaredInterfaceType()},
-            signatureLoc));
-        return;
-
-      case Symbol::Kind::Layout:
-        errors.push_back(RequirementError::forConflictingRequirement(
-            {RequirementKind::Layout, subjectType, constraint.getLayoutConstraint()},
-            signatureLoc));
-        return;
-
-      case Symbol::Kind::ConcreteConformance:
-      case Symbol::Kind::AssociatedType:
-      case Symbol::Kind::GenericParam:
-      case Symbol::Kind::Name:
-        return;
-      }
-    };
-
-    if (firstProperty->getKind() == Symbol::Kind::ConcreteType) {
-      recordError(*firstProperty, *secondProperty);
-    } else if (secondProperty->getKind() == Symbol::Kind::ConcreteType) {
-      recordError(*secondProperty, *firstProperty);
-    }
+    errors.push_back(RequirementError::forConflictingRequirement(
+        *firstRule->getPropertyRequirement(subject),
+        *secondRule->getPropertyRequirement(subject),
+        signatureLoc));
   }
 }
 
diff --git a/lib/AST/RequirementMachine/Rule.cpp b/lib/AST/RequirementMachine/Rule.cpp
@@ -43,6 +43,33 @@ Optional<Symbol> Rule::isPropertyRule() const {
   return property;
 }
 
+Optional<Requirement> Rule::getPropertyRequirement(Type subject) const {
+  auto property = isPropertyRule();
+  if (!property)
+    return None;
+
+  switch (property->getKind()) {
+  case Symbol::Kind::ConcreteType:
+    return Requirement(RequirementKind::SameType, subject,
+                       property->getConcreteType());
+
+  case Symbol::Kind::Superclass:
+    return Requirement(RequirementKind::Superclass, subject,
+                       property->getConcreteType());
+
+  case Symbol::Kind::Protocol:
+    return Requirement(RequirementKind::Conformance, subject,
+                       property->getProtocol()->getDeclaredInterfaceType());
+
+  case Symbol::Kind::Layout:
+    return Requirement(RequirementKind::Layout, subject,
+                       property->getLayoutConstraint());
+
+  default:
+    return None;
+  }
+}
+
 /// If this is a rule of the form T.[P] => T where [P] is a protocol symbol,
 /// return the protocol P, otherwise return nullptr.
 const ProtocolDecl *Rule::isProtocolConformanceRule() const {
diff --git a/lib/AST/RequirementMachine/Rule.h b/lib/AST/RequirementMachine/Rule.h
@@ -13,6 +13,8 @@
 #ifndef SWIFT_RULE_H
 #define SWIFT_RULE_H
 
+#include "swift/AST/Requirement.h"
+
 #include "Symbol.h"
 #include "Term.h"
 
@@ -107,6 +109,8 @@ class Rule final {
 
   Optional<Symbol> isPropertyRule() const;
 
+  Optional<Requirement> getPropertyRequirement(Type subject) const;
+
   const ProtocolDecl *isProtocolConformanceRule() const;
 
   const ProtocolDecl *isAnyConformanceRule() const;
diff --git a/test/Generics/requirement_machine_diagnostics.swift b/test/Generics/requirement_machine_diagnostics.swift
@@ -210,7 +210,7 @@ protocol ProtoAlias2 {
 }
 
 func basicConflict<T: ProtoAlias1 & ProtoAlias2>(_:T) where T.A1 == T.A2 {}
-// expected-error@-1{{generic signature requires types 'Int' and 'String' to be the same}}
+// expected-error@-1{{no type for 'T.A1' can satisfy both 'T.A1 == Int' and 'T.A1 == String'}}
 
 protocol RequiresAnyObject {
   associatedtype A: AnyObject
@@ -228,17 +228,17 @@ protocol RequiresSuperclass {
 func testMissingRequirements() {
   struct S {}
   func conflict1<T: RequiresAnyObject>(_: T) where T.A == S {}
-  // expected-error@-1{{same-type constraint type 'S' does not conform to required protocol 'AnyObject'}}
+  // expected-error@-1{{no type for 'T.A' can satisfy both 'T.A == S' and 'T.A : AnyObject'}}
 
   func conflict2<T: RequiresConformance>(_: T) where T.A == C {}
-  // expected-error@-1{{same-type constraint type 'C' does not conform to required protocol 'P'}}
+  // expected-error@-1{{no type for 'T.A' can satisfy both 'T.A : P' and 'T.A == C'}}
 
   class C {}
   func conflict3<T: RequiresSuperclass>(_: T) where T.A == C {}
-  // expected-error@-1{{type 'T.A' cannot be a subclass of both 'Super' and 'C'}}
+  // expected-error@-1{{no type for 'T.A' can satisfy both 'T.A : Super' and 'T.A : C'}}
 
   func conflict4<T: RequiresSuperclass>(_: T) where T.A: C {}
-  // expected-error@-1{{type 'T.A' cannot be a subclass of both 'Super' and 'C'}}
+  // expected-error@-1{{no type for 'T.A' can satisfy both 'T.A : Super' and 'T.A : C'}}
 }
 
 protocol Fooable {
@@ -265,44 +265,44 @@ func sameTypeConflicts() {
     var bar: Y { return Y() }
   }
 
-  // expected-error@+1{{generic parameter 'T.Foo' cannot be equal to both 'Y' and 'X'}}
+  // expected-error@+1{{no type for 'T.Foo' can satisfy both 'T.Foo == Y' and 'T.Foo == X'}}
   func fail1<
     T: Fooable, U: Fooable
   >(_ t: T, u: U) -> (X, Y)
     where T.Foo == X, U.Foo == Y, T.Foo == U.Foo {
     fatalError()
   }
 
-  // expected-error@+1{{generic parameter 'T.Foo' cannot be equal to both 'X' and 'Y'}}
+  // expected-error@+1{{no type for 'T.Foo' can satisfy both 'T.Foo == X' and 'T.Foo == Y'}}
   func fail2<
     T: Fooable, U: Fooable
   >(_ t: T, u: U) -> (X, Y)
     where T.Foo == U.Foo, T.Foo == X, U.Foo == Y {
     fatalError()
   }
 
-  // expected-error@+1{{same-type constraint type 'X' does not conform to required protocol 'Fooable'}}
+  // expected-error@+1{{no type for 'T.Bar' can satisfy both 'T.Bar : Fooable' and 'T.Bar == X'}}
   func fail3<T: Barrable>(_ t: T) -> X
     where T.Bar == X {
     fatalError()
   }
 
-  // expected-error@+1{{generic parameter 'T.Bar.Foo' cannot be equal to both 'Z' and 'X'}}
+  // expected-error@+1{{no type for 'T.Bar.Foo' can satisfy both 'T.Bar.Foo == Z' and 'T.Bar.Foo == X'}}
   func fail4<T: Barrable>(_ t: T) -> (Y, Z)
     where
     T.Bar == Y,
     T.Bar.Foo == Z {
     fatalError()
   }
 
-  // expected-error@+1{{generic parameter 'T.Bar.Foo' cannot be equal to both 'Z' and 'X'}}
+  // expected-error@+1{{no type for 'T.Bar.Foo' can satisfy both 'T.Bar.Foo == Z' and 'T.Bar.Foo == X'}}
   func fail5<T: Barrable>(_ t: T) -> (Y, Z)
     where
     T.Bar.Foo == Z,
     T.Bar == Y {
     fatalError()
   }
 
-  // expected-error@+1{{generic parameter 'T.X' cannot be equal to both 'Int' and 'String'}}
+  // expected-error@+1{{no type for 'T.X' can satisfy both 'T.X == Int' and 'T.X == String'}}
   func fail6<U, T: Concrete>(_: U, _: T) where T.X == String {}
 }
