Skip to content

Commit

Permalink
[clang] p2085 out-of-class comparison operator defaulting
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@urnathan
urnathan committed Dec 16, 2021
1 parent 5aefb1d commit 5fbe21a
Show file tree
Hide file tree
Showing 5 changed files with 221 additions and 81 deletions.
13 changes: 10 additions & 3 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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">;
Expand Down
202 changes: 133 additions & 69 deletions clang/lib/Sema/SemaDeclCXX.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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())
Expand All @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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() &&
Expand All @@ -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) {
Expand Down
47 changes: 39 additions & 8 deletions clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
View file
Original file line number Diff line number Diff line change
@@ -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}}
Expand All @@ -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'}}
Expand Down Expand Up @@ -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
38 changes: 38 additions & 0 deletions clang/test/CodeGenCXX/p2085.cpp
View file
Original file line number Diff line number Diff line change
@@ -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_
Loading

0 comments on commit 5fbe21a

Please sign in to comment.