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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Thanks, this looks fine. I wish we had a better way to determine whether a builtin comparison is usable without actually building a use of it. :-(

In D95409#2565227 <https://reviews.llvm.org/D95409#2565227>, @mizvekov wrote:

> By the way I realize I may not have put the new tests in the appropriate place. Any guidance there welcome.

I've suggested better locations.



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:7680-7681
       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
----------------
Please can you extend the FIXME comment below to call out that relational comparisons on a function pointer is the one known case where there is a builtin candidate that can't actually be used?


================
Comment at: clang/test/CXX/class/class.compare/class.compare.default/p6.cpp:1
+// RUN: %clang_cc1 -std=c++2a -verify %s
+
----------------
Hm, I don't think this is the right place for this test. The `A` test is a test for [class.compare.default]p2 (which disallows reference members). The `B`, `C`, and `D` tests are for [class.eq]p2 (which requires `xi == xi` to be a usable expression). Can you move the tests to those places?


================
Comment at: clang/test/CXX/class/class.compare/class.spaceship/p4.cpp:19
+  auto operator<=>(A const &) const = default; // expected-warning {{explicitly defaulted three-way comparison operator is implicitly deleted}}
+  const int &a = 0;                            // expected-note {{defaulted 'operator<=>' is implicitly deleted because class 'A' has a reference member}}
+};
----------------
Similar to above, this is [class.compare.default]p2.


================
Comment at: clang/test/CXX/class/class.compare/class.spaceship/p4.cpp:34
+  auto operator<=>(D const &) const = default; // expected-warning {{explicitly defaulted three-way comparison operator is implicitly deleted}}
+  int D::*a;                                   // expected-note {{defaulted 'operator<=>' is implicitly deleted because there is no viable comparison function for member 'a'}}
+};
----------------
... and these are [class.spaceship]p2 ("The operator function is defined as deleted if that expression is not usable")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95409