[clang] 4a8530f - [clang] implicitly delete space ship operator with function pointers

[Richard Smith via cfe-commits]

Author: Matheus Izvekov
Date: 2021-02-26T16:03:01-08:00
New Revision: 4a8530fc3039f128eddc38737f0172bb3d489bcf

URL: https://github.com/llvm/llvm-project/commit/4a8530fc3039f128eddc38737f0172bb3d489bcf
DIFF: https://github.com/llvm/llvm-project/commit/4a8530fc3039f128eddc38737f0172bb3d489bcf.diff

LOG: [clang] implicitly delete space ship operator with function pointers

See bug #48856

Definitions of classes with member function pointers and default
spaceship operator were getting accepted with no diagnostic on
release build, and triggering assert on builds with runtime checks
enabled. Diagnostics were only produced when actually comparing
instances of such classes.

This patch makes it so Spaceship and Less operators are not considered
as builtin operator candidates for function pointers, producing
equivalent diagnostics for the cases where pointers to member function
and pointers to data members are used instead.

Reviewed By: rsmith

Differential Revision: https://reviews.llvm.org/D95409

Added: 
    

Modified: 
    clang/lib/Sema/SemaDeclCXX.cpp
    clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
    clang/test/CXX/class/class.compare/class.eq/p2.cpp
    clang/test/CXX/class/class.compare/class.spaceship/p2.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index cb35f0fb848f3..a150bc9fd766a 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7685,10 +7685,14 @@ class DefaultedComparisonAnalyzer
 
     if (Args[0]->getType()->isOverloadableType())
       S.LookupOverloadedBinOp(CandidateSet, OO, Fns, Args);
-    else {
+    else if (OO == OO_EqualEqual ||
+             !Args[0]->getType()->isFunctionPointerType()) {
       // FIXME: We determine whether this is a valid expression by checking to
       // see if there's a viable builtin operator candidate for it. That isn't
       // really what the rules ask us to do, but should give the right results.
+      //
+      // Note that the builtin operator for relational comparisons on function
+      // pointers is the only known case which cannot be used.
       S.AddBuiltinOperatorCandidates(OO, FD->getLocation(), Args, CandidateSet);
     }
 

diff  --git a/clang/test/CXX/class/class.compare/class.compare.default/p2.cpp b/clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
index cdffd445f7cea..226245ce8a440 100644
--- a/clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
+++ b/clang/test/CXX/class/class.compare/class.compare.default/p2.cpp
@@ -39,6 +39,14 @@ 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 at -1 {{because there is no viable comparison function}}
+};
+
 struct B1 {
   struct {
     int x;

diff  --git a/clang/test/CXX/class/class.compare/class.eq/p2.cpp b/clang/test/CXX/class/class.compare/class.eq/p2.cpp
index e5f4a020d5c83..7e9416574eeea 100644
--- a/clang/test/CXX/class/class.compare/class.eq/p2.cpp
+++ b/clang/test/CXX/class/class.compare/class.eq/p2.cpp
@@ -15,6 +15,25 @@ struct E {
 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 at -1 {{because there is no viable comparison function}}
+  void (*x)();
+};
+struct H2 {
+  bool operator==(const H2 &) const = default;
+  bool operator<(const H2 &) const = default; // expected-warning {{implicitly deleted}}
+  // expected-note at -1 {{because there is no viable comparison function}}
+  void (H2::*x)();
+};
+struct H3 {
+  bool operator==(const H3 &) const = default;
+  bool operator<(const H3 &) const = default; // expected-warning {{implicitly deleted}}
+  // expected-note at -1 {{because there is no viable comparison function}}
+  int H3::*x;
+};
+
 template<typename T> struct X {
   X();
   bool operator==(const X&) const = default; // #x expected-note 4{{deleted here}}

diff  --git a/clang/test/CXX/class/class.compare/class.spaceship/p2.cpp b/clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
index dae31e925ba1e..d7f95ee4a6fde 100644
--- a/clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
+++ b/clang/test/CXX/class/class.compare/class.spaceship/p2.cpp
@@ -155,3 +155,20 @@ namespace BadDeducedType {
     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'}}
+  };
+
+  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'}}
+  };
+
+  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'}}
+  };
+}