[clang] Improve diagnostics on implicitly deleted defaulted comparisons

[mizvekov]

Summary

This patch just makes the error message clearer by reinforcing the cause
was a lack of viable three-way comparison function for the
complete object.

Signed-off-by: Matheus Izvekov <[e-mail]>

[rsmith]

I don't think this change is right: for

struct A {
  int &r;
  bool operator==(const A&) const = default;
};

we should warn about the reference member (as we do), but for

struct A {
  int &r;
  bool operator<(const A&) const = default;
};

... we shouldn't recurse to subobjects because they make no difference. The problem in that example is that there's no operator<=> for A; the fact that there's a reference member is irrelevant. I think it would be useful to change the diagnostic from

<stdin>:3:8: warning: explicitly defaulted relational comparison operator is implicitly deleted [-Wdefaulted-function-deleted]
  bool operator<(const A &) const = default;
       ^
<stdin>:3:8: note: defaulted 'operator<' is implicitly deleted because there is no viable comparison function

to something more verbose, such as

<stdin>:3:8: warning: explicitly defaulted relational comparison operator is implicitly deleted [-Wdefaulted-function-deleted]
  bool operator<(const A &) const = default;
       ^
<stdin>:3:8: note: defaulted 'operator<' is implicitly deleted because there is no viable three-way comparison function for 'A'

[mizvekov]

Now this a patch just about improving the diagnostic as rsmith suggested.

[mizvekov]

You are absolutely right, my bad!!

So we stealth fix this revision to improve the message :D

[mizvekov]

Change to simpler way to do it:
For CompleteObject Kind, just fill in the declaration for the complete object.

[rsmith]

clang/include/clang/Basic/DiagnosticSemaKinds.td

8968–8970

Would it be useful to apply the same diagnostic improvement to this diagnostic too? (Genuine question: I *think* we'll attach a note pointing to the deleted function in this case, which would probably make the "for T" part just be noise, but I've not checked.)

[mizvekov]

clang/include/clang/Basic/DiagnosticSemaKinds.td

8968–8970

Yeah I thought about checking other errors for similar improvements. It's on my list to check that.

[mizvekov]

clang/include/clang/Basic/DiagnosticSemaKinds.td

8968–8970

For this one in particular, we really do attach another note that makes it obvious, pointing to the explicitly marked deleted function.
note_defaulted_comparison_not_constexpr at first glance looks like might suffer from the same problem, but in this case I don't think the error ever could apply to the complete object.
And then nothing else that I could find.

clang/include/clang/Basic/DiagnosticSemaKinds.td

def note_defaulted_comparison_reference_member : Note<
   "class %1 has a reference member">;
 def note_defaulted_comparison_ambiguous : Note<
   "defaulted %0 is implicitly deleted because implied %select{|'==' |'<' }1"
   "comparison %select{|for member %3 |for base class %3 }2is ambiguous">;
 def note_defaulted_comparison_inaccessible : Note<
   "defaulted %0 is implicitly deleted because it would invoke a "
   "%select{private|protected}3 %4%select{ member of %6|"
   " member of %6 to compare member %2| to compare base class %2}1">;
 def note_defaulted_comparison_calls_deleted : Note<
   "defaulted %0 is implicitly deleted because it would invoke a deleted "
   "comparison function%select{| for member %2| for base class %2}1">;
 def note_defaulted_comparison_no_viable_function : Note<
-  "defaulted %0 is implicitly deleted because there is no viable comparison "
-  "function%select{| for member %2| for base class %2}1">;
+  "defaulted %0 is implicitly deleted because there is no viable three-way "
+  "comparison function for%select{| member| base class}1 %2">;
 def note_defaulted_comparison_no_viable_function_synthesized : Note<
   "three-way comparison cannot be synthesized because there is no viable "
   "function for %select{'=='|'<'}0 comparison">;
 def note_defaulted_comparison_not_rewritten_callee : Note<
   "defaulted %0 is implicitly deleted because this non-rewritten comparison "
   "function would be the best match for the comparison">;
 def note_defaulted_comparison_cannot_deduce : Note<
   "return type of defaulted 'operator<=>' cannot be deduced because "

clang/lib/Sema/SemaDeclCXX.cpp

case OR_No_Viable_Function:
         if (!R.add(visitBinaryOperator(OO_EqualEqual, Args, Subobj,
                                        &CandidateSet)))
           R.add(visitBinaryOperator(OO_Less, Args, Subobj, &CandidateSet));
         break;
       }
 
       if (Diagnose == ExplainDeleted) {
         S.Diag(Subobj.Loc, diag::note_defaulted_comparison_no_viable_function)
-            << FD << Subobj.Kind << Subobj.Decl;
+            << FD << Subobj.Kind
+            << (Subobj.Kind != DefaultedComparisonSubobject::CompleteObject
+                    ? Subobj.Decl
+                    : RD);
 
         // For a three-way comparison, list both the candidates for the
         // original operator and the candidates for the synthesized operator.
         if (SpaceshipCandidates) {
           SpaceshipCandidates->NoteCandidates(
               S, Args,
               SpaceshipCandidates->CompleteCandidates(S, OCD_AllCandidates,
                                                       Args, FD->getLocation()));

clang/test/CXX/class/class.compare/class.compare.default/p1.cpp

namespace LookupContext {
   }
 }
 
 namespace P1946 {
   struct A {
     friend bool operator==(A &, A &); // expected-note {{would lose const qualifier}}
   };
   struct B {
-    A a; // expected-note {{no viable comparison}}
+    A a; // expected-note {{no viable three-way comparison}}
     friend bool operator==(B, B) = default; // ok
     friend bool operator==(const B&, const B&) = default; // expected-warning {{deleted}}
   };
 }

clang/test/CXX/class/class.compare/class.compare.default/p2.cpp

void f(A2 a) {
   void(a >= a);
 }
 
 struct A3 {
   int &x; // expected-note {{because class 'A3' has a reference member}}
 
   bool operator==(const A3 &) const = default; // expected-warning {{implicitly deleted}}
   bool operator<(const A3 &) const = default;  // expected-warning {{implicitly deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
+  // expected-note@-1 {{because there is no viable three-way comparison function for 'A3'}}
 };
 
 struct B1 {
   struct {
     int x;
     int &y; // expected-note 2{{because class 'B1' has a reference member}}
   };
 

clang/test/CXX/class/class.compare/class.compare.default/p4.cpp

void use_j(J j) {
   void(j == j); // expected-error {{ambiguous}}
 }
 
 namespace DeleteAfterFirstDecl {
   bool operator==(const struct Q&, const struct Q&);
   struct Q {
     struct X {
       friend std::strong_ordering operator<=>(const X&, const X&);
-    } x; // expected-note {{no viable comparison}}
+    } x; // expected-note {{no viable three-way comparison}}
     // expected-error@+1 {{defaulting the corresponding implicit 'operator==' for this defaulted 'operator<=>' would delete it after its first declaration}}
     friend std::strong_ordering operator<=>(const Q&, const Q&) = default;
   };
 }
 
 // Note, substitution here results in the second parameter of 'operator=='
 // referring to the first parameter of 'operator==', not to the first parameter
 // of 'operator<=>'.

clang/test/CXX/class/class.compare/class.eq/p2.cpp

struct E {
   int operator==(E) const; // expected-note {{passing}}
 };
 struct F { void operator==(F) const; };
 struct G { bool operator==(G) const = delete; }; // expected-note {{deleted here}}
 
 struct H1 {
   bool operator==(const H1 &) const = default;
   bool operator<(const H1 &) const = default; // expected-warning {{implicitly deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
+  // expected-note@-1 {{because there is no viable three-way comparison function for 'H1'}}
   void (*x)();
 };
 struct H2 {
   bool operator==(const H2 &) const = default;
   bool operator<(const H2 &) const = default; // expected-warning {{implicitly deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
+  // expected-note@-1 {{because there is no viable three-way comparison function for 'H2'}}
   void (H2::*x)();
 };
 struct H3 {
   bool operator==(const H3 &) const = default;
   bool operator<(const H3 &) const = default; // expected-warning {{implicitly deleted}}
-  // expected-note@-1 {{because there is no viable comparison function}}
+  // expected-note@-1 {{because there is no viable three-way comparison function for 'H3'}}
   int H3::*x;
 };
 
 template<typename T> struct X {
   X();
   bool operator==(const X&) const = default; // #x expected-note 4{{deleted here}}
-  T t; // expected-note 3{{because there is no viable comparison function for member 't'}}
+  T t; // expected-note 3{{because there is no viable three-way comparison function for member 't'}}
        // expected-note@-1 {{because it would invoke a deleted comparison function for member 't'}}
 };
 
 struct Mutable {
   bool operator==(const Mutable&) const = default;
   mutable D d;
 };
 

clang/test/CXX/class/class.compare/class.spaceship/p1.cpp

struct G2 {
     void operator==(const G2&) const;
     bool operator<(const G2&) const;
   };
   struct H {
     void operator<=>(const H&) const;
   };
 
   // expected-note@#base {{deleted comparison function for base class 'C'}}
-  // expected-note@#base {{no viable comparison function for base class 'D1'}}
+  // expected-note@#base {{no viable three-way comparison function for base class 'D1'}}
   // expected-note@#base {{three-way comparison cannot be synthesized because there is no viable function for '<' comparison}}
-  // expected-note@#base {{no viable comparison function for base class 'D2'}}
+  // expected-note@#base {{no viable three-way comparison function for base class 'D2'}}
   // expected-note@#base {{three-way comparison cannot be synthesized because there is no viable function for '==' comparison}}
   // expected-note@#base {{deleted comparison function for base class 'E'}}
   // expected-note@#base {{implied comparison for base class 'F' is ambiguous}}
   template<typename T> struct Cmp : T { // #base
     std::strong_ordering operator<=>(const Cmp&) const = default; // #cmp expected-note 5{{here}}
   };
 
   void use(...);
use(
       Cmp<G2>() <=> Cmp<G2>(), // expected-note-re {{in defaulted three-way comparison operator for '{{.*}}Cmp<{{.*}}G2>' first required here}}j
       // expected-error@#cmp {{no matching conversion for static_cast from 'void' to 'std::strong_ordering'}}
       Cmp<H>() <=> Cmp<H>(), // expected-note-re {{in defaulted three-way comparison operator for '{{.*}}Cmp<{{.*}}H>' first required here}}j
       0
     );
   }
 
   // expected-note@#arr {{deleted comparison function for member 'arr'}}
-  // expected-note@#arr {{no viable comparison function for member 'arr'}}
+  // expected-note@#arr {{no viable three-way comparison function for member 'arr'}}
   // expected-note@#arr {{three-way comparison cannot be synthesized because there is no viable function for '<' comparison}}
-  // expected-note@#arr {{no viable comparison function for member 'arr'}}
+  // expected-note@#arr {{no viable three-way comparison function for member 'arr'}}
   // expected-note@#arr {{three-way comparison cannot be synthesized because there is no viable function for '==' comparison}}
   // expected-note@#arr {{deleted comparison function for member 'arr'}}
   // expected-note@#arr {{implied comparison for member 'arr' is ambiguous}}
   template<typename T> struct CmpArray {
     T arr[3]; // #arr
     std::strong_ordering operator<=>(const CmpArray&) const = default; // #cmparray expected-note 5{{here}}
   };
   void g() {

clang/test/CXX/class/class.compare/class.spaceship/p2.cpp

 }
 
 namespace DeducedVsSynthesized {
   struct A {
     bool operator==(const A&) const;
     bool operator<(const A&) const;
   };
   struct B {
-    A a; // expected-note {{no viable comparison function for member 'a'}}
+    A a; // expected-note {{no viable three-way comparison function for member 'a'}}
     auto operator<=>(const B&) const = default; // expected-warning {{implicitly deleted}}
   };
 }
 
 namespace Deduction {
   template<typename T> struct wrap {
     T t;
     friend auto operator<=>(const wrap&, const wrap&) = default;
struct D {
     // expected-error@+1 {{deduced return type for defaulted three-way comparison operator must be 'auto', not 'CmpCat auto'}}
     friend CmpCat auto operator<=>(const D&, const D&) = default;
   };
 }
 
 namespace PR48856 {
   struct A {
     auto operator<=>(const A &) const = default; // expected-warning {{implicitly deleted}}
-    void (*x)();                                 // expected-note {{because there is no viable comparison function for member 'x'}}
+    void (*x)();                                 // expected-note {{because there is no viable three-way comparison function for member 'x'}}
   };
 
   struct B {
     auto operator<=>(const B &) const = default; // expected-warning {{implicitly deleted}}
-    void (B::*x)();                              // expected-note {{because there is no viable comparison function for member 'x'}}
+    void (B::*x)();                              // expected-note {{because there is no viable three-way comparison function for member 'x'}}
   };
 
   struct C {
     auto operator<=>(const C &) const = default; // expected-warning {{implicitly deleted}}
-    int C::*x;                                   // expected-note {{because there is no viable comparison function for member 'x'}}
+    int C::*x;                                   // expected-note {{because there is no viable three-way comparison function for member 'x'}}
   };
 }