[clang] 5fbe21a - [clang] p2085 out-of-class comparison operator defaulting
Nathan Sidwell via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 16 07:22:58 PST 2021
Author: Nathan Sidwell
Date: 2021-12-16T07:22:46-08:00
New Revision: 5fbe21a7748f91adbd1b16c95bbfe180642320a3
URL: https://github.com/llvm/llvm-project/commit/5fbe21a7748f91adbd1b16c95bbfe180642320a3
DIFF: https://github.com/llvm/llvm-project/commit/5fbe21a7748f91adbd1b16c95bbfe180642320a3.diff
LOG: [clang] p2085 out-of-class comparison operator defaulting
This implements p2085, allowing out-of-class defaulting of comparison
operators, primarily so they need not be inline, IIUC intent. this was
mostly straigh forward, but required reimplementing
Sema::CheckExplicitlyDefaultedComparison, as now there's a case where
we have no a priori clue as to what class a defaulted comparison may
be for. We have to inspect the parameter types to find out. Eg:
class X { ... };
bool operator==(X, X) = default;
Thus reimplemented the parameter type checking, and added 'is this a
friend' functionality for the above case.
Reviewed By: mizvekov
Differential Revision: https://reviews.llvm.org/D104478
Added:
clang/test/CodeGenCXX/p2085.cpp
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
clang/www/cxx_status.html
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 038067521559c..29e7c5b447140 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9151,15 +9151,22 @@ def warn_cxx17_compat_defaulted_comparison : Warning<
"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_num_args : Error<
+ "%select{non-member|member}0 %sub{select_defaulted_comparison_kind}1"
+ " comparison operator must have %select{2|1}0 parameters">;
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_param_unknown : Error<
+ "invalid parameter type for non-member defaulted"
+ " %sub{select_defaulted_comparison_kind}0"
+ "; found %1, expected class or reference to a constant class">;
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">;
+def err_defaulted_comparison_not_friend : Error<
+ "%sub{select_defaulted_comparison_kind}0 is not a friend of"
+ " %select{|incomplete class }1%2">;
def err_defaulted_comparison_non_const : Error<
"defaulted member %sub{select_defaulted_comparison_kind}0 must be "
"const-qualified">;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 3f6bde1b9ed6a..119cdf2a3d3c0 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -8435,9 +8435,6 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
DefaultedComparisonKind DCK) {
assert(DCK != DefaultedComparisonKind::None && "not a defaulted comparison");
- CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(FD->getLexicalDeclContext());
- assert(RD && "defaulted comparison is not defaulted in a class");
-
// Perform any unqualified lookups we're going to need to default this
// function.
if (S) {
@@ -8455,43 +8452,17 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
// const C&, or
// -- a friend of C having two parameters of type const C& or two
// parameters of type C.
- QualType ExpectedParmType1 = Context.getRecordType(RD);
- QualType ExpectedParmType2 =
- Context.getLValueReferenceType(ExpectedParmType1.withConst());
- if (isa<CXXMethodDecl>(FD))
- ExpectedParmType1 = ExpectedParmType2;
- for (const ParmVarDecl *Param : FD->parameters()) {
- if (!Param->getType()->isDependentType() &&
- !Context.hasSameType(Param->getType(), ExpectedParmType1) &&
- !Context.hasSameType(Param->getType(), ExpectedParmType2)) {
- // Don't diagnose an implicit 'operator=='; we will have diagnosed the
- // corresponding defaulted 'operator<=>' already.
- if (!FD->isImplicit()) {
- Diag(FD->getLocation(), diag::err_defaulted_comparison_param)
- << (int)DCK << Param->getType() << ExpectedParmType1
- << !isa<CXXMethodDecl>(FD)
- << ExpectedParmType2 << Param->getSourceRange();
- }
- 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();
- }
- return true;
- }
- // ... non-static const member ...
- if (auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+ CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(FD->getLexicalDeclContext());
+ bool IsMethod = isa<CXXMethodDecl>(FD);
+ if (IsMethod) {
+ auto *MD = cast<CXXMethodDecl>(FD);
assert(!MD->isStatic() && "comparison function cannot be a static member");
+
+ // If we're out-of-class, this is the class we're comparing.
+ if (!RD)
+ RD = MD->getParent();
+
if (!MD->isConst()) {
SourceLocation InsertLoc;
if (FunctionTypeLoc Loc = MD->getFunctionTypeLoc())
@@ -8500,7 +8471,7 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
// corresponding defaulted 'operator<=>' already.
if (!MD->isImplicit()) {
Diag(MD->getLocation(), diag::err_defaulted_comparison_non_const)
- << (int)DCK << FixItHint::CreateInsertion(InsertLoc, " const");
+ << (int)DCK << FixItHint::CreateInsertion(InsertLoc, " const");
}
// Add the 'const' to the type to recover.
@@ -8510,9 +8481,100 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
MD->setType(Context.getFunctionType(FPT->getReturnType(),
FPT->getParamTypes(), EPI));
}
- } else {
- // A non-member function declared in a class must be a friend.
+ }
+
+ if (FD->getNumParams() != (IsMethod ? 1 : 2)) {
+ // Let's not worry about using a variadic template pack here -- who would do
+ // such a thing?
+ Diag(FD->getLocation(), diag::err_defaulted_comparison_num_args)
+ << int(IsMethod) << int(DCK);
+ return true;
+ }
+
+ const ParmVarDecl *KnownParm = nullptr;
+ for (const ParmVarDecl *Param : FD->parameters()) {
+ QualType ParmTy = Param->getType();
+ if (ParmTy->isDependentType())
+ continue;
+ if (!KnownParm) {
+ auto CTy = ParmTy;
+ // Is it `T const &`?
+ bool Ok = !IsMethod;
+ QualType ExpectedTy;
+ if (RD)
+ ExpectedTy = Context.getRecordType(RD);
+ if (auto *Ref = CTy->getAs<ReferenceType>()) {
+ CTy = Ref->getPointeeType();
+ if (RD)
+ ExpectedTy.addConst();
+ Ok = true;
+ }
+
+ // Is T a class?
+ if (!Ok) {
+ } else if (RD) {
+ if (!RD->isDependentType() && !Context.hasSameType(CTy, ExpectedTy))
+ Ok = false;
+ } else if (auto *CRD = CTy->getAsRecordDecl()) {
+ RD = cast<CXXRecordDecl>(CRD);
+ } else {
+ Ok = false;
+ }
+
+ if (Ok) {
+ KnownParm = Param;
+ } else {
+ // Don't diagnose an implicit 'operator=='; we will have diagnosed the
+ // corresponding defaulted 'operator<=>' already.
+ if (!FD->isImplicit()) {
+ if (RD) {
+ QualType PlainTy = Context.getRecordType(RD);
+ QualType RefTy =
+ Context.getLValueReferenceType(PlainTy.withConst());
+ if (IsMethod)
+ PlainTy = QualType();
+ Diag(FD->getLocation(), diag::err_defaulted_comparison_param)
+ << int(DCK) << ParmTy << RefTy << int(!IsMethod) << PlainTy
+ << Param->getSourceRange();
+ } else {
+ assert(!IsMethod && "should know expected type for method");
+ Diag(FD->getLocation(),
+ diag::err_defaulted_comparison_param_unknown)
+ << int(DCK) << ParmTy << Param->getSourceRange();
+ }
+ }
+ return true;
+ }
+ } else if (!Context.hasSameType(KnownParm->getType(), ParmTy)) {
+ Diag(FD->getLocation(), diag::err_defaulted_comparison_param_mismatch)
+ << int(DCK) << KnownParm->getType() << KnownParm->getSourceRange()
+ << ParmTy << Param->getSourceRange();
+ return true;
+ }
+ }
+
+ assert(RD && "must have determined class");
+ if (IsMethod) {
+ } else if (isa<CXXRecordDecl>(FD->getLexicalDeclContext())) {
+ // In-class, must be a friend decl.
assert(FD->getFriendObjectKind() && "expected a friend declaration");
+ } else {
+ // Out of class, require the defaulted comparison to be a friend (of a
+ // complete type).
+ if (RequireCompleteType(FD->getLocation(), Context.getRecordType(RD),
+ diag::err_defaulted_comparison_not_friend, int(DCK),
+ int(1)))
+ return true;
+
+ if (llvm::find_if(RD->friends(), [&](const FriendDecl *F) {
+ return FD->getCanonicalDecl() ==
+ F->getFriendDecl()->getCanonicalDecl();
+ }) == RD->friends().end()) {
+ Diag(FD->getLocation(), diag::err_defaulted_comparison_not_friend)
+ << int(DCK) << int(0) << RD;
+ Diag(RD->getCanonicalDecl()->getLocation(), diag::note_declared_at);
+ return true;
+ }
}
// C++2a [class.eq]p1, [class.rel]p1:
@@ -8670,7 +8732,10 @@ void Sema::DefineDefaultedComparison(SourceLocation UseLoc, FunctionDecl *FD,
{
// Build and set up the function body.
- CXXRecordDecl *RD = cast<CXXRecordDecl>(FD->getLexicalParent());
+ // The first parameter has type maybe-ref-to maybe-const T, use that to get
+ // the type of the class being compared.
+ auto PT = FD->getParamDecl(0)->getType();
+ CXXRecordDecl *RD = PT.getNonReferenceType()->getAsCXXRecordDecl();
SourceLocation BodyLoc =
FD->getEndLoc().isValid() ? FD->getEndLoc() : FD->getLocation();
StmtResult Body =
@@ -17180,13 +17245,6 @@ void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation DefaultLoc) {
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.
if (DefKind.isComparison() &&
@@ -17208,31 +17266,37 @@ void Sema::SetDeclDefaulted(Decl *Dcl, SourceLocation DefaultLoc) {
// that we've marked it as defaulted.
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())
+ // If this is a comparison's defaulted definition within the record, do
+ // the checking when the record is complete.
+ if (DefKind.isComparison() && isa<CXXRecordDecl>(FD->getLexicalDeclContext()))
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
- // '= default' on it.
- Primary = Pattern;
-
- // If the method was defaulted on its first declaration, we will have
+ // If this member fn was defaulted on its first declaration, we will have
// already performed the checking in CheckCompletedCXXClass. Such a
// declaration doesn't trigger an implicit definition.
- if (Primary->getCanonicalDecl()->isDefaulted())
- return;
+ if (isa<CXXMethodDecl>(FD)) {
+ const FunctionDecl *Primary = FD;
+ if (const FunctionDecl *Pattern = FD->getTemplateInstantiationPattern())
+ // Ask the template instantiation pattern that actually had the
+ // '= default' on it.
+ Primary = Pattern;
+ if (Primary->getCanonicalDecl()->isDefaulted())
+ return;
+ }
- // FIXME: Once we support defining comparisons out of class, check for a
- // defaulted comparison here.
- if (CheckExplicitlyDefaultedSpecialMember(MD, DefKind.asSpecialMember()))
- MD->setInvalidDecl();
- else
- DefineDefaultedFunction(*this, MD, DefaultLoc);
+ if (DefKind.isComparison()) {
+ if (CheckExplicitlyDefaultedComparison(nullptr, FD, DefKind.asComparison()))
+ FD->setInvalidDecl();
+ else
+ DefineDefaultedComparison(DefaultLoc, FD, DefKind.asComparison());
+ } else {
+ auto *MD = cast<CXXMethodDecl>(FD);
+
+ if (CheckExplicitlyDefaultedSpecialMember(MD, DefKind.asSpecialMember()))
+ MD->setInvalidDecl();
+ else
+ DefineDefaultedFunction(*this, MD, DefaultLoc);
+ }
}
static void SearchForReturnInStmt(Sema &Self, Stmt *S) {
diff --git a/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp b/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
index dd622988d4585..136afd8996432 100644
--- a/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ b/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -1,15 +1,13 @@
// 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}}
struct A {
friend bool operator==(const A&, const A&) = default;
- friend bool operator!=(const A&, const B&) = default; // expected-error {{invalid parameter type for defaulted equality comparison operator; found 'const B &', expected 'A' or 'const A &'}}
+ friend bool operator!=(const A&, const B&) = default; // expected-error {{parameters for defaulted equality comparison operator must have the same type (found 'const A &' vs 'const B &')}}
friend bool operator!=(const B&, const B&) = default; // expected-error {{invalid parameter type for defaulted equality comparison}}
friend bool operator<(const A&, const A&);
friend bool operator<(const B&, const B&) = default; // expected-error {{invalid parameter type for defaulted relational comparison}}
@@ -29,11 +27,6 @@ struct A {
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,41 @@ namespace P1946 {
friend bool operator==(const B&, const B&) = default; // expected-warning {{deleted}}
};
}
+
+namespace p2085 {
+// out-of-class defaulting
+
+struct S1 {
+ bool operator==(S1 const &) const;
+};
+
+bool S1::operator==(S1 const &) const = default;
+
+bool F1(S1 &s) {
+ return s != s;
+}
+
+struct S2 {
+ friend bool operator==(S2 const &, S2 const &);
+};
+
+bool operator==(S2 const &, S2 const &) = default;
+bool F2(S2 &s) {
+ return s != s;
+}
+
+struct S3 {}; // expected-note{{here}}
+bool operator==(S3 const &, S3 const &) = default; // expected-error{{not a friend}}
+
+struct S4; // expected-note{{forward declaration}}
+bool operator==(S4 const &, S4 const &) = default; // expected-error{{not a friend}}
+
+struct S5; // expected-note 3{{forward declaration}}
+bool operator==(S5, S5) = default; // expected-error{{not a friend}} expected-error 2{{has incomplete type}}
+
+enum e {};
+bool operator==(e, int) = default; // expected-error{{expected class or reference to a constant class}}
+
+bool operator==(e *, int *) = default; // expected-error{{must have at least one}}
+
+} // namespace p2085
diff --git a/clang/test/CodeGenCXX/p2085.cpp b/clang/test/CodeGenCXX/p2085.cpp
new file mode 100644
index 0000000000000..40f02ac6b3e73
--- /dev/null
+++ b/clang/test/CodeGenCXX/p2085.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 --std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s
+
+namespace std {
+struct strong_ordering {
+ int n;
+ constexpr operator int() const { return n; }
+ static const strong_ordering equal, greater, less;
+};
+constexpr inline strong_ordering strong_ordering::equal = {0};
+constexpr inline strong_ordering strong_ordering::greater = {1};
+constexpr inline strong_ordering strong_ordering::less = {-1};
+} // namespace std
+
+struct Space {
+ int i, j;
+
+ std::strong_ordering operator<=>(Space const &other) const;
+ bool operator==(Space const &other) const;
+};
+
+// Make sure these cause emission
+std::strong_ordering Space::operator<=>(Space const &other) const = default;
+// CHECK-LABEL: define{{.*}} @_ZNK5SpacessERKS_
+bool Space::operator==(Space const &) const = default;
+// CHECK-LABEL: define{{.*}} @_ZNK5SpaceeqERKS_
+
+struct Water {
+ int i, j;
+
+ std::strong_ordering operator<=>(Water const &other) const;
+ bool operator==(Water const &other) const;
+};
+
+// Make sure these do not cause emission
+inline std::strong_ordering Water::operator<=>(Water const &other) const = default;
+// CHECK-NOT: define{{.*}} @_ZNK5WaterssERKS_
+inline bool Water::operator==(Water const &) const = default;
+// CHECK-NOT: define{{.*}} @_ZNK5WatereqERKS_
diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index 10ba777be648c..3cf12ff477215 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -1006,7 +1006,7 @@ <h2 id="cxx20">C++20 implementation status</h2>
</tr>
<tr>
<td><a href="https://wg21.link/p2085r0">P2085R0</a></td>
- <td class="none" align="center">No</td>
+ <td class="unreleased" align="center">Clang 14</td>
</tr>
<tr>
<td>Access checking on specializations</td>
More information about the cfe-commits
mailing list