Skip to content

Commit c7df775

Browse files
MitalAshokMitalAshok
authored
[Clang] Check explicit object parameter for defaulted operators properly (#100419)
Previously, the type of explicit object parameters was not considered for relational operators. This was defined by CWG2586, <https://cplusplus.github.io/CWG/issues/2586.html>. This fix also means CWG2547 <https://cplusplus.github.io/CWG/issues/2547.html> is now fully implemented. Fixes #100329, fixes #104413. Now start rejecting invalid rvalue reference parameters, which weren't checked for, and start accepting non-reference explicit object parameters (like `bool operator==(this C, C) = default;`) which were previously rejected for the object param not being a reference. Also start rejecting non-reference explicit object parameters for defaulted copy/move assign operators (`A& operator=(this A, const A&) = default;` is invalid but was previously accepted). Fixes #104414.
1 parent fcefe95 commit c7df775

File tree

7 files changed

+136
-28
lines changed

7 files changed

+136
-28
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,9 @@ Bug Fixes to C++ Support
254254
specialization of a conversion function template.
255255
- Correctly diagnose attempts to use a concept name in its own definition;
256256
A concept name is introduced to its scope sooner to match the C++ standard. (#GH55875)
257+
- Properly reject defaulted relational operators with invalid types for explicit object parameters,
258+
e.g., ``bool operator==(this int, const Foo&)`` (#GH100329), and rvalue reference parameters.
259+
- Properly reject defaulted copy/move assignment operators that have a non-reference explicit object parameter.
257260

258261
Bug Fixes to AST Handling
259262
^^^^^^^^^^^^^^^^^^^^^^^^^

‎clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9752,7 +9752,7 @@ def err_defaulted_special_member_quals : Error<
97529752
"have 'const'%select{, 'constexpr'|}1 or 'volatile' qualifiers">;
97539753
def err_defaulted_special_member_explicit_object_mismatch : Error<
97549754
"the type of the explicit object parameter of an explicitly-defaulted "
9755-
"%select{copy|move}0 assignment operator should match the type of the class %1">;
9755+
"%select{copy|move}0 assignment operator should be reference to %1">;
97569756
def err_defaulted_special_member_volatile_param : Error<
97579757
"the parameter for an explicitly-defaulted %sub{select_special_member_kind}0 "
97589758
"may not be volatile">;

‎clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 28 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7644,9 +7644,13 @@ bool Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
76447644
// parameter is of (possibly different) type “reference to C”,
76457645
// in which case the type of F1 would differ from the type of F2
76467646
// in that the type of F1 has an additional parameter;
7647-
if (!Context.hasSameType(
7648-
ThisType.getNonReferenceType().getUnqualifiedType(),
7649-
Context.getRecordType(RD))) {
7647+
QualType ExplicitObjectParameter = MD->isExplicitObjectMemberFunction()
7648+
? MD->getParamDecl(0)->getType()
7649+
: QualType();
7650+
if (!ExplicitObjectParameter.isNull() &&
7651+
(!ExplicitObjectParameter->isReferenceType() ||
7652+
!Context.hasSameType(ExplicitObjectParameter.getNonReferenceType(),
7653+
Context.getRecordType(RD)))) {
76507654
if (DeleteOnTypeMismatch)
76517655
ShouldDeleteForTypeMismatch = true;
76527656
else {
@@ -8730,8 +8734,9 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
87308734
// If we're out-of-class, this is the class we're comparing.
87318735
if (!RD)
87328736
RD = MD->getParent();
8733-
QualType T = MD->getFunctionObjectParameterType();
8734-
if (!T.isConstQualified()) {
8737+
QualType T = MD->getFunctionObjectParameterReferenceType();
8738+
if (!T.getNonReferenceType().isConstQualified() &&
8739+
(MD->isImplicitObjectMemberFunction() || T->isLValueReferenceType())) {
87358740
SourceLocation Loc, InsertLoc;
87368741
if (MD->isExplicitObjectMemberFunction()) {
87378742
Loc = MD->getParamDecl(0)->getBeginLoc();
@@ -8750,11 +8755,17 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
87508755
}
87518756

87528757
// Add the 'const' to the type to recover.
8753-
const auto *FPT = MD->getType()->castAs<FunctionProtoType>();
8754-
FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
8755-
EPI.TypeQuals.addConst();
8756-
MD->setType(Context.getFunctionType(FPT->getReturnType(),
8757-
FPT->getParamTypes(), EPI));
8758+
if (MD->isExplicitObjectMemberFunction()) {
8759+
assert(T->isLValueReferenceType());
8760+
MD->getParamDecl(0)->setType(Context.getLValueReferenceType(
8761+
T.getNonReferenceType().withConst()));
8762+
} else {
8763+
const auto *FPT = MD->getType()->castAs<FunctionProtoType>();
8764+
FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
8765+
EPI.TypeQuals.addConst();
8766+
MD->setType(Context.getFunctionType(FPT->getReturnType(),
8767+
FPT->getParamTypes(), EPI));
8768+
}
87588769
}
87598770

87608771
if (MD->isVolatile()) {
@@ -8781,33 +8792,27 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
87818792

87828793
const ParmVarDecl *KnownParm = nullptr;
87838794
for (const ParmVarDecl *Param : FD->parameters()) {
8784-
if (Param->isExplicitObjectParameter())
8785-
continue;
87868795
QualType ParmTy = Param->getType();
8787-
87888796
if (!KnownParm) {
87898797
auto CTy = ParmTy;
87908798
// Is it `T const &`?
8791-
bool Ok = !IsMethod;
8799+
bool Ok = !IsMethod || FD->hasCXXExplicitFunctionObjectParameter();
87928800
QualType ExpectedTy;
87938801
if (RD)
87948802
ExpectedTy = Context.getRecordType(RD);
8795-
if (auto *Ref = CTy->getAs<ReferenceType>()) {
8803+
if (auto *Ref = CTy->getAs<LValueReferenceType>()) {
87968804
CTy = Ref->getPointeeType();
87978805
if (RD)
87988806
ExpectedTy.addConst();
87998807
Ok = true;
88008808
}
88018809

88028810
// Is T a class?
8803-
if (!Ok) {
8804-
} else if (RD) {
8805-
if (!RD->isDependentType() && !Context.hasSameType(CTy, ExpectedTy))
8806-
Ok = false;
8807-
} else if (auto *CRD = CTy->getAsRecordDecl()) {
8808-
RD = cast<CXXRecordDecl>(CRD);
8811+
if (RD) {
8812+
Ok &= RD->isDependentType() || Context.hasSameType(CTy, ExpectedTy);
88098813
} else {
8810-
Ok = false;
8814+
RD = CTy->getAsCXXRecordDecl();
8815+
Ok &= RD != nullptr;
88118816
}
88128817

88138818
if (Ok) {
@@ -8847,7 +8852,7 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
88478852
assert(FD->getFriendObjectKind() && "expected a friend declaration");
88488853
} else {
88498854
// Out of class, require the defaulted comparison to be a friend (of a
8850-
// complete type).
8855+
// complete type, per CWG2547).
88518856
if (RequireCompleteType(FD->getLocation(), Context.getRecordType(RD),
88528857
diag::err_defaulted_comparison_not_friend, int(DCK),
88538858
int(1)))

‎clang/test/CXX/class/class.compare/class.compare.default/p1.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ struct A {
1616
bool operator<(const A&) const;
1717
bool operator<=(const A&) const = default;
1818
bool operator==(const A&) const && = default; // expected-error {{ref-qualifier '&&' is not allowed on a defaulted comparison operator}}
19+
bool operator<=(const A&&) const = default; // expected-error {{invalid parameter type for defaulted relational comparison operator; found 'const A &&', expected 'const A &'}}
20+
bool operator<=(const int&) const = default; // expected-error {{invalid parameter type for defaulted relational comparison operator; found 'const int &', expected 'const A &'}}
1921
bool operator>=(const A&) const volatile = default; // expected-error {{defaulted comparison function must not be volatile}}
2022
bool operator<=>(const A&) = default; // expected-error {{defaulted member three-way comparison operator must be const-qualified}}
2123
bool operator>=(const B&) const = default; // expected-error-re {{invalid parameter type for defaulted relational comparison operator; found 'const B &', expected 'const A &'{{$}}}}

‎clang/test/CXX/drs/cwg25xx.cpp

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,30 @@ using ::cwg2521::operator""_div;
9292
#endif
9393
} // namespace cwg2521
9494

95+
namespace cwg2547 { // cwg2547: 20
96+
#if __cplusplus >= 202302L
97+
struct S;
98+
// since-cxx23-note@-1 {{forward declaration of 'cwg2547::S'}}
99+
// since-cxx23-note@-2 {{forward declaration of 'cwg2547::S'}}
100+
// since-cxx23-note@-3 {{forward declaration of 'cwg2547::S'}}
101+
bool operator==(S, S) = default; // error: S is not complete
102+
// since-cxx23-error@-1 {{variable has incomplete type 'S'}}
103+
// since-cxx23-error@-2 {{variable has incomplete type 'S'}}
104+
// since-cxx23-error@-3 {{equality comparison operator is not a friend of incomplete class 'cwg2547::S'}}
105+
struct S {
106+
friend bool operator==(S, const S&) = default; // error: parameters of different types
107+
// since-cxx23-error@-1 {{parameters for defaulted equality comparison operator must have the same type (found 'S' vs 'const S &')}}
108+
};
109+
enum E { };
110+
bool operator==(E, E) = default; // error: not a member or friend of a class
111+
// since-cxx23-error@-1 {{invalid parameter type for non-member defaulted equality comparison operator; found 'E', expected class or reference to a constant class}}
112+
113+
struct S2 {
114+
bool operator==(this int, S2) = default;
115+
// since-cxx23-error@-1 {{invalid parameter type for defaulted equality comparison operator; found 'int', expected 'const cwg2547::S2 &'}}
116+
};
117+
#endif
118+
} // namespace cwg2547
95119

96120
#if __cplusplus >= 202302L
97121
namespace cwg2553 { // cwg2553: 18 review 2023-07-14
@@ -253,6 +277,41 @@ static_assert(__is_layout_compatible(U, V), "");
253277
#endif
254278
} // namespace cwg2583
255279

280+
namespace cwg2586 { // cwg2586: 20
281+
#if __cplusplus >= 202302L
282+
struct X {
283+
X& operator=(this X&, const X&) = default;
284+
X& operator=(this X&, X&) = default;
285+
X& operator=(this X&&, X&&) = default;
286+
// FIXME: The notes could be clearer on *how* the type differs
287+
// e.g., "if an explicit object parameter is used it must be of type reference to 'X'"
288+
X& operator=(this int, const X&) = default;
289+
// since-cxx23-warning@-1 {{explicitly defaulted copy assignment operator is implicitly deleted}}
290+
// since-cxx23-note@-2 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
291+
X& operator=(this X, const X&) = default;
292+
// since-cxx23-warning@-1 {{explicitly defaulted copy assignment operator is implicitly deleted}}
293+
// since-cxx23-note@-2 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
294+
};
295+
struct Y {
296+
void operator=(this int, const Y&); // This is copy constructor, suppresses implicit declaration
297+
};
298+
static_assert([]<typename T = Y>{
299+
return !requires(T t, const T& ct) { t = ct; };
300+
}());
301+
302+
struct Z {
303+
bool operator==(this const Z&, const Z&) = default;
304+
bool operator==(this Z, Z) = default;
305+
bool operator==(this Z, const Z&) = default;
306+
// since-cxx23-error@-1 {{parameters for defaulted equality comparison operator must have the same type (found 'Z' vs 'const Z &')}}
307+
bool operator==(this const Z&, Z) = default;
308+
// since-cxx23-error@-1 {{parameters for defaulted equality comparison operator must have the same type (found 'const Z &' vs 'Z')}}
309+
bool operator==(this int, Z) = default;
310+
// since-cxx23-error@-1 {{invalid parameter type for defaulted equality comparison operator; found 'int', expected 'const cwg2586::Z &'}}
311+
};
312+
#endif
313+
} // namespace cwg2586
314+
256315
namespace cwg2598 { // cwg2598: 18
257316
#if __cplusplus >= 201103L
258317
struct NonLiteral {

‎clang/test/SemaCXX/cxx2b-deducing-this.cpp

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -729,10 +729,10 @@ struct S2 {
729729
};
730730

731731
S2& S2::operator=(this int&& self, const S2&) = default;
732-
// expected-error@-1 {{the type of the explicit object parameter of an explicitly-defaulted copy assignment operator should match the type of the class 'S2'}}
732+
// expected-error@-1 {{the type of the explicit object parameter of an explicitly-defaulted copy assignment operator should be reference to 'S2'}}
733733

734734
S2& S2::operator=(this int&& self, S2&&) = default;
735-
// expected-error@-1 {{the type of the explicit object parameter of an explicitly-defaulted move assignment operator should match the type of the class 'S2'}}
735+
// expected-error@-1 {{the type of the explicit object parameter of an explicitly-defaulted move assignment operator should be reference to 'S2'}}
736736

737737
struct Move {
738738
Move& operator=(this int&, Move&&) = default;
@@ -972,3 +972,42 @@ struct R {
972972
f(r_value_ref); // expected-error {{no matching member function for call to 'f'}}
973973
}
974974
};
975+
976+
namespace GH100329 {
977+
struct A {
978+
bool operator == (this const int&, const A&);
979+
};
980+
bool A::operator == (this const int&, const A&) = default;
981+
// expected-error@-1 {{invalid parameter type for defaulted equality comparison operator; found 'const int &', expected 'const GH100329::A &'}}
982+
} // namespace GH100329
983+
984+
namespace defaulted_assign {
985+
struct A {
986+
A& operator=(this A, const A&) = default;
987+
// expected-warning@-1 {{explicitly defaulted copy assignment operator is implicitly deleted}}
988+
// expected-note@-2 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
989+
A& operator=(this int, const A&) = default;
990+
// expected-warning@-1 {{explicitly defaulted copy assignment operator is implicitly deleted}}
991+
// expected-note@-2 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
992+
};
993+
} // namespace defaulted_assign
994+
995+
namespace defaulted_compare {
996+
struct A {
997+
bool operator==(this A&, const A&) = default;
998+
// expected-error@-1 {{defaulted member equality comparison operator must be const-qualified}}
999+
bool operator==(this const A, const A&) = default;
1000+
// expected-error@-1 {{invalid parameter type for defaulted equality comparison operator; found 'const A', expected 'const defaulted_compare::A &'}}
1001+
bool operator==(this A, A) = default;
1002+
};
1003+
struct B {
1004+
int a;
1005+
bool operator==(this B, B) = default;
1006+
};
1007+
static_assert(B{0} == B{0});
1008+
static_assert(B{0} != B{1});
1009+
template<B b>
1010+
struct X;
1011+
static_assert(__is_same(X<B{0}>, X<B{0}>));
1012+
static_assert(!__is_same(X<B{0}>, X<B{1}>));
1013+
} // namespace defaulted_compare

‎clang/www/cxx_dr_status.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15097,7 +15097,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
1509715097
<td><a href="https://cplusplus.github.io/CWG/issues/2547.html">2547</a></td>
1509815098
<td>DRWP</td>
1509915099
<td>Defaulted comparison operator function for non-classes</td>
15100-
<td class="unknown" align="center">Unknown</td>
15100+
<td class="unreleased" align="center">Clang 20</td>
1510115101
</tr>
1510215102
<tr id="2548">
1510315103
<td><a href="https://cplusplus.github.io/CWG/issues/2548.html">2548</a></td>
@@ -15331,7 +15331,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
1533115331
<td><a href="https://cplusplus.github.io/CWG/issues/2586.html">2586</a></td>
1533215332
<td>CD6</td>
1533315333
<td>Explicit object parameter for assignment and comparison</td>
15334-
<td class="unknown" align="center">Unknown</td>
15334+
<td class="unreleased" align="center">Clang 20</td>
1533515335
</tr>
1533615336
<tr class="open" id="2587">
1533715337
<td><a href="https://cplusplus.github.io/CWG/issues/2587.html">2587</a></td>

0 commit comments

Comments
 (0)