predator5047 updated this revision to Diff 357747.
predator5047 added a comment.

Addressed  Richard's comments.

Fix handling of friend functions.
Added an error dialog for when enum type are used as params.
Removed define outside of class dialog since it is no longer needed.
Moved tests to p1.cpp and added more tests.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103929/new/

https://reviews.llvm.org/D103929

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===================================================================
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -996,7 +996,7 @@
       </tr>
       <tr>
         <td><a href="https://wg21.link/p2085r0";>P2085R0</a></td>
-        <td class="none" align="center">No</td>
+        <td class="none" align="center">Clang 13</td>
       </tr>
     <tr>
       <td>Access checking on specializations</td>
Index: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
===================================================================
--- clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -1,8 +1,6 @@
 // RUN: %clang_cc1 -std=c++2a -verify %s
 
 struct B {};
-bool operator==(const B&, const B&) = default; // expected-error {{equality comparison operator can only be defaulted in a class definition}} expected-note {{candidate}}
-bool operator<=>(const B&, const B&) = default; // expected-error {{three-way comparison operator can only be defaulted in a class definition}}
 
 template<typename T = void>
   bool operator<(const B&, const B&) = default; // expected-error {{comparison operator template cannot be defaulted}}
@@ -29,11 +27,6 @@
     bool operator==(const A&) const = default; // expected-error {{comparison operator template cannot be defaulted}}
 };
 
-// FIXME: The wording is not clear as to whether these are valid, but the
-// intention is that they are not.
-bool operator<(const A&, const A&) = default; // expected-error {{relational comparison operator can only be defaulted in a class definition}}
-bool A::operator<(const A&) const = default; // expected-error {{can only be defaulted in a class definition}}
-
 template<typename T> struct Dependent {
   using U = typename T::type;
   bool operator==(U) const = default; // expected-error {{found 'Dependent<Bad>::U'}}
@@ -132,3 +125,98 @@
     friend bool operator==(const B&, const B&) = default; // expected-warning {{deleted}}
   };
 }
+
+namespace P2085 {
+
+// Test defaulted ref params
+struct A {
+  bool operator==(const A &) const;
+  friend bool operator!=(const A &, const A &);
+};
+
+bool A::operator==(const A &) const = default;
+bool operator!=(const A &, const A &) = default;
+
+// Test defaulted value params
+
+struct B {
+  friend bool operator==(B, B);
+  bool operator==(B) const;
+};
+bool operator==(B, B) = default;
+
+bool B::operator==(B) const = default; // expected-error {{invalid parameter type for defaulted equality comparison operator; found 'P2085::B', expected 'const P2085::B &'}}
+
+// Test for friend of C but not friend of A
+struct C {
+  friend bool operator==(const A &, const A &);
+};
+bool operator==(const A &, const A &) = default; //  expected-error {{defaulted 'operator==' is not a friend of 'P2085::A'}}
+
+struct D {
+  struct E {
+    bool operator==(const E &) const = default;
+    friend bool operator!=(const E &, const E &);
+  };
+};
+
+bool operator!=(const D::E &, const D::E &) = default;
+
+template <class T>
+struct F {
+  T __Ignore;
+  bool operator==(const F &) const = default;
+  friend bool operator!=(const F &, const F &);
+};
+
+const bool R0 = F<int>{} == F<int>{};
+
+template <class T>
+bool operator!=(const F<T> &, const F<T> &) = default; // expected-error {{comparison operator template cannot be defaulted}}
+
+struct G {
+};
+
+bool operator!=(const G &, const G &) = default; // expected-error  {{defaulted 'operator!=' is not a friend of 'P2085::G'}}
+
+enum foo { a,
+           b };
+
+struct H {
+  bool operator==(foo) const;
+};
+
+bool H::operator==(foo) const = default; //expected-error {{invalid parameter type for defaulted equality comparison operator; found 'P2085::foo', expected 'const P2085::H &'}}
+
+struct K {
+  friend bool operator==(foo, K);
+  friend bool operator==(K, foo);
+};
+
+bool operator==(foo, K) = default;                   //expected-error {{parameters for defaulted equality comparison operator must have the same type (found 'P2085::foo' vs 'P2085::K')}}
+bool operator==(K, foo) = default;                   //expected-error {{parameters for defaulted equality comparison operator must have the same type (found 'P2085::K' vs 'P2085::foo')}}
+bool operator==(foo, foo) = default;                 //expected-error {{argument 'P2085::foo' is not of class type}}
+bool operator==(const foo &, const foo &) = default; //expected-error {{argument 'const P2085::foo &' is not of class type}}
+
+struct L {
+
+  bool operator==(const A &) const;
+  int operator<=>(const A &) const;
+};
+
+bool L::operator==(const A &) const = default; //expected-error {{invalid parameter type for defaulted equality comparison operator; found 'const P2085::A &', expected 'const P2085::L &'}}
+int L::operator<=>(const A &) const = default; //expected-error {{invalid parameter type for defaulted three-way comparison operator; found 'const P2085::A &', expected 'const P2085::L &'}}
+
+template <char C> struct Broken {
+  template <typename T = void> friend bool operator==(Broken, Broken) { return T::result; } //expected-error {{type 'void'}}
+};
+
+struct Q {
+  Broken<'B'> b;
+  friend bool operator!=(const Q &, const Q &) = default; //expected-no-error
+  friend bool operator==(const Q &, const Q &);
+};
+
+bool operator==(const Q &, const Q &) = default; // expected-note {{in instantiation of}}
+
+} // namespace P2085
\ No newline at end of file
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -753,11 +753,14 @@
             << MD->isExplicitlyDefaulted() << DFK.asSpecialMember()
             << Context.getTagDeclType(MD->getParent());
       } else if (DFK.isComparison()) {
+        auto RD = FD->getParamDecl(0)
+                      ->getType()
+                      .getNonReferenceType()
+                      .getUnqualifiedType()
+                      ->getAsCXXRecordDecl();
         Diags.Report(Active->PointOfInstantiation,
                      diag::note_comparison_synthesized_at)
-            << (int)DFK.asComparison()
-            << Context.getTagDeclType(
-                   cast<CXXRecordDecl>(FD->getLexicalDeclContext()));
+            << (int)DFK.asComparison() << Context.getTagDeclType(RD);
       }
       break;
     }
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -8370,9 +8370,70 @@
 bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
                                               DefaultedComparisonKind DCK) {
   assert(DCK != DefaultedComparisonKind::None && "not a defaulted comparison");
+  CXXRecordDecl *RD =
+      dyn_cast_or_null<CXXRecordDecl>(FD->getLexicalDeclContext());
+  if (!RD) {
+    if (auto *MD = dyn_cast_or_null<CXXMethodDecl>(FD)) {
+      RD = MD->getParent();
+    }
+  }
+  auto ParamsHaveSameType = [&] {
+    if (!Context.hasSameType(FD->getParamDecl(0)->getType(),
+                             FD->getParamDecl(1)->getType())) {
+      if (!FD->isImplicit()) {
+        Diag(FD->getLocation(), diag::err_defaulted_comparison_param_mismatch)
+            << (int)DCK << FD->getParamDecl(0)->getType()
+            << FD->getParamDecl(0)->getSourceRange()
+            << FD->getParamDecl(1)->getType()
+            << FD->getParamDecl(1)->getSourceRange();
+      }
+      return false;
+    }
+    return true;
+  };
+
+  if (!RD) {
+    if (!ParamsHaveSameType()) {
+      return true;
+    }
+    auto Param0 = FD->getParamDecl(0)
+                      ->getType()
+                      .getNonReferenceType()
+                      .getUnqualifiedType();
+
+    RD = Param0->getAsCXXRecordDecl();
+
+    if (!RD) {
+      Diag(FD->getLocation(),
+           diag::err_defaulted_comparison_function_not_cxx_record)
+          << FD << FD->getParamDecl(0)->getType()
+          << FD->getParamDecl(0)->getSourceRange();
+      return true;
+    }
+
+    llvm::SmallSet<FunctionDecl *, 8> FriendFunctions;
+
+    for (const auto &I : RD->friends()) {
+      FriendFunctions.insert(
+          dyn_cast_or_null<FunctionDecl>(I->getFriendDecl()));
+    }
+
+    bool IsFriend = false;
+    for (const auto &I : FD->redecls()) {
+      if (FriendFunctions.count(I)) {
+        IsFriend = true;
+        break;
+      }
+    }
 
-  CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(FD->getLexicalDeclContext());
-  assert(RD && "defaulted comparison is not defaulted in a class");
+    if (!IsFriend) {
+      Diag(FD->getLocation(),
+           diag::err_defaulted_comparison_function_not_friend)
+          << FD << Param0;
+
+      return true;
+    }
+  }
 
   // Perform any unqualified lookups we're going to need to default this
   // function.
@@ -8411,17 +8472,7 @@
       return true;
     }
   }
-  if (FD->getNumParams() == 2 &&
-      !Context.hasSameType(FD->getParamDecl(0)->getType(),
-                           FD->getParamDecl(1)->getType())) {
-    if (!FD->isImplicit()) {
-      Diag(FD->getLocation(), diag::err_defaulted_comparison_param_mismatch)
-          << (int)DCK
-          << FD->getParamDecl(0)->getType()
-          << FD->getParamDecl(0)->getSourceRange()
-          << FD->getParamDecl(1)->getType()
-          << FD->getParamDecl(1)->getSourceRange();
-    }
+  if (FD->getNumParams() == 2 && !ParamsHaveSameType()) {
     return true;
   }
 
@@ -8446,9 +8497,6 @@
       MD->setType(Context.getFunctionType(FPT->getReturnType(),
                                           FPT->getParamTypes(), EPI));
     }
-  } else {
-    // A non-member function declared in a class must be a friend.
-    assert(FD->getFriendObjectKind() && "expected a friend declaration");
   }
 
   // C++2a [class.eq]p1, [class.rel]p1:
@@ -8606,7 +8654,12 @@
 
   {
     // Build and set up the function body.
-    CXXRecordDecl *RD = cast<CXXRecordDecl>(FD->getLexicalParent());
+
+    CXXRecordDecl *RD = FD->getParamDecl(0)
+                            ->getType()
+                            .getNonReferenceType()
+                            .getUnqualifiedType()
+                            ->getAsCXXRecordDecl();
     SourceLocation BodyLoc =
         FD->getEndLoc().isValid() ? FD->getEndLoc() : FD->getLocation();
     StmtResult Body =
@@ -17078,12 +17131,6 @@
     return;
   }
 
-  if (DefKind.isComparison() &&
-      !isa<CXXRecordDecl>(FD->getLexicalDeclContext())) {
-    Diag(FD->getLocation(), diag::err_defaulted_comparison_out_of_class)
-        << (int)DefKind.asComparison();
-    return;
-  }
 
   // Issue compatibility warning. We already warned if the operator is
   // 'operator<=>' when parsing the '<=>' token.
@@ -17107,12 +17154,18 @@
   FD->setWillHaveBody(false);
 
   // If this definition appears within the record, do the checking when
-  // the record is complete. This is always the case for a defaulted
-  // comparison.
-  if (DefKind.isComparison())
+  // the record is complete. If it's defined outside the record define
+  // it now.
+  if (DefKind.isComparison()) {
+    if (!isa<CXXRecordDecl>(FD->getLexicalDeclContext()) &&
+        !CheckExplicitlyDefaultedComparison(getCurScope(), FD,
+                                            DefKind.asComparison())) {
+
+      DefineDefaultedFunction(*this, FD, DefaultLoc);
+    }
     return;
+  }
   auto *MD = cast<CXXMethodDecl>(FD);
-
   const FunctionDecl *Primary = FD;
   if (const FunctionDecl *Pattern = FD->getTemplateInstantiationPattern())
     // Ask the template instantiation pattern that actually had the
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9090,12 +9090,13 @@
   "before C++20">, InGroup<CXXPre20Compat>, DefaultIgnore;
 def err_defaulted_comparison_template : Error<
   "comparison operator template cannot be defaulted">;
-def err_defaulted_comparison_out_of_class : Error<
-  "%sub{select_defaulted_comparison_kind}0 can only be defaulted in a class "
-  "definition">;
 def err_defaulted_comparison_param : Error<
   "invalid parameter type for defaulted %sub{select_defaulted_comparison_kind}0"
   "; found %1, expected %2%select{| or %4}3">;
+def err_defaulted_comparison_function_not_friend : Error<
+  "defaulted %0 is not a friend of %1">;
+def err_defaulted_comparison_function_not_cxx_record : Error<
+  "argument %1 is not of class type">;
 def err_defaulted_comparison_param_mismatch : Error<
   "parameters for defaulted %sub{select_defaulted_comparison_kind}0 "
   "must have the same type%diff{ (found $ vs $)|}1,2">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D103929:... Alexandru Octavian Buțiu via Phabricator via cfe-commits
    • [PATCH] D10... Alexandru Octavian Buțiu via Phabricator via cfe-commits
    • [PATCH] D10... Richard Smith - zygoloid via Phabricator via cfe-commits
    • [PATCH] D10... Alexandru Octavian Buțiu via Phabricator via cfe-commits
    • [PATCH] D10... Alexandru Octavian Buțiu via Phabricator via cfe-commits
    • [PATCH] D10... Alexandru Octavian Buțiu via Phabricator via cfe-commits
    • [PATCH] D10... Alexandru Octavian Buțiu via Phabricator via cfe-commits

Reply via email to