[clang] c7df775 - [Clang] Check explicit object parameter for defaulted operators properly (#100419)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 15 08:12:14 PDT 2024
Author: Mital Ashok
Date: 2024-08-15T19:12:11+04:00
New Revision: c7df775440717ec5a3f47b6d485617d802cbd036
URL: https://github.com/llvm/llvm-project/commit/c7df775440717ec5a3f47b6d485617d802cbd036
DIFF: https://github.com/llvm/llvm-project/commit/c7df775440717ec5a3f47b6d485617d802cbd036.diff
LOG: [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.
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
clang/test/CXX/drs/cwg25xx.cpp
clang/test/SemaCXX/cxx2b-deducing-this.cpp
clang/www/cxx_dr_status.html
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f5696d6ce15dc7..b6b7dd5705637a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -254,6 +254,9 @@ Bug Fixes to C++ Support
specialization of a conversion function template.
- Correctly diagnose attempts to use a concept name in its own definition;
A concept name is introduced to its scope sooner to match the C++ standard. (#GH55875)
+- Properly reject defaulted relational operators with invalid types for explicit object parameters,
+ e.g., ``bool operator==(this int, const Foo&)`` (#GH100329), and rvalue reference parameters.
+- Properly reject defaulted copy/move assignment operators that have a non-reference explicit object parameter.
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index da2f939067bfab..461eeb19f65e4a 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9752,7 +9752,7 @@ def err_defaulted_special_member_quals : Error<
"have 'const'%select{, 'constexpr'|}1 or 'volatile' qualifiers">;
def err_defaulted_special_member_explicit_object_mismatch : Error<
"the type of the explicit object parameter of an explicitly-defaulted "
- "%select{copy|move}0 assignment operator should match the type of the class %1">;
+ "%select{copy|move}0 assignment operator should be reference to %1">;
def err_defaulted_special_member_volatile_param : Error<
"the parameter for an explicitly-defaulted %sub{select_special_member_kind}0 "
"may not be volatile">;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index c87edf41f35cee..9ca91a2def39f5 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7644,9 +7644,13 @@ bool Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
// parameter is of (possibly
diff erent) type “reference to C”,
// in which case the type of F1 would
diff er from the type of F2
// in that the type of F1 has an additional parameter;
- if (!Context.hasSameType(
- ThisType.getNonReferenceType().getUnqualifiedType(),
- Context.getRecordType(RD))) {
+ QualType ExplicitObjectParameter = MD->isExplicitObjectMemberFunction()
+ ? MD->getParamDecl(0)->getType()
+ : QualType();
+ if (!ExplicitObjectParameter.isNull() &&
+ (!ExplicitObjectParameter->isReferenceType() ||
+ !Context.hasSameType(ExplicitObjectParameter.getNonReferenceType(),
+ Context.getRecordType(RD)))) {
if (DeleteOnTypeMismatch)
ShouldDeleteForTypeMismatch = true;
else {
@@ -8730,8 +8734,9 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
// If we're out-of-class, this is the class we're comparing.
if (!RD)
RD = MD->getParent();
- QualType T = MD->getFunctionObjectParameterType();
- if (!T.isConstQualified()) {
+ QualType T = MD->getFunctionObjectParameterReferenceType();
+ if (!T.getNonReferenceType().isConstQualified() &&
+ (MD->isImplicitObjectMemberFunction() || T->isLValueReferenceType())) {
SourceLocation Loc, InsertLoc;
if (MD->isExplicitObjectMemberFunction()) {
Loc = MD->getParamDecl(0)->getBeginLoc();
@@ -8750,11 +8755,17 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
}
// Add the 'const' to the type to recover.
- const auto *FPT = MD->getType()->castAs<FunctionProtoType>();
- FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
- EPI.TypeQuals.addConst();
- MD->setType(Context.getFunctionType(FPT->getReturnType(),
- FPT->getParamTypes(), EPI));
+ if (MD->isExplicitObjectMemberFunction()) {
+ assert(T->isLValueReferenceType());
+ MD->getParamDecl(0)->setType(Context.getLValueReferenceType(
+ T.getNonReferenceType().withConst()));
+ } else {
+ const auto *FPT = MD->getType()->castAs<FunctionProtoType>();
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+ EPI.TypeQuals.addConst();
+ MD->setType(Context.getFunctionType(FPT->getReturnType(),
+ FPT->getParamTypes(), EPI));
+ }
}
if (MD->isVolatile()) {
@@ -8781,18 +8792,15 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
const ParmVarDecl *KnownParm = nullptr;
for (const ParmVarDecl *Param : FD->parameters()) {
- if (Param->isExplicitObjectParameter())
- continue;
QualType ParmTy = Param->getType();
-
if (!KnownParm) {
auto CTy = ParmTy;
// Is it `T const &`?
- bool Ok = !IsMethod;
+ bool Ok = !IsMethod || FD->hasCXXExplicitFunctionObjectParameter();
QualType ExpectedTy;
if (RD)
ExpectedTy = Context.getRecordType(RD);
- if (auto *Ref = CTy->getAs<ReferenceType>()) {
+ if (auto *Ref = CTy->getAs<LValueReferenceType>()) {
CTy = Ref->getPointeeType();
if (RD)
ExpectedTy.addConst();
@@ -8800,14 +8808,11 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
}
// 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);
+ if (RD) {
+ Ok &= RD->isDependentType() || Context.hasSameType(CTy, ExpectedTy);
} else {
- Ok = false;
+ RD = CTy->getAsCXXRecordDecl();
+ Ok &= RD != nullptr;
}
if (Ok) {
@@ -8847,7 +8852,7 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD,
assert(FD->getFriendObjectKind() && "expected a friend declaration");
} else {
// Out of class, require the defaulted comparison to be a friend (of a
- // complete type).
+ // complete type, per CWG2547).
if (RequireCompleteType(FD->getLocation(), Context.getRecordType(RD),
diag::err_defaulted_comparison_not_friend, int(DCK),
int(1)))
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 ddf82f432c2eab..a195e0548152d6 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
@@ -16,6 +16,8 @@ struct A {
bool operator<(const A&) const;
bool operator<=(const A&) const = default;
bool operator==(const A&) const && = default; // expected-error {{ref-qualifier '&&' is not allowed on a defaulted comparison operator}}
+ bool operator<=(const A&&) const = default; // expected-error {{invalid parameter type for defaulted relational comparison operator; found 'const A &&', expected 'const A &'}}
+ bool operator<=(const int&) const = default; // expected-error {{invalid parameter type for defaulted relational comparison operator; found 'const int &', expected 'const A &'}}
bool operator>=(const A&) const volatile = default; // expected-error {{defaulted comparison function must not be volatile}}
bool operator<=>(const A&) = default; // expected-error {{defaulted member three-way comparison operator must be const-qualified}}
bool operator>=(const B&) const = default; // expected-error-re {{invalid parameter type for defaulted relational comparison operator; found 'const B &', expected 'const A &'{{$}}}}
diff --git a/clang/test/CXX/drs/cwg25xx.cpp b/clang/test/CXX/drs/cwg25xx.cpp
index 0d9f5eac23866a..1924008f15ba58 100644
--- a/clang/test/CXX/drs/cwg25xx.cpp
+++ b/clang/test/CXX/drs/cwg25xx.cpp
@@ -92,6 +92,30 @@ using ::cwg2521::operator""_div;
#endif
} // namespace cwg2521
+namespace cwg2547 { // cwg2547: 20
+#if __cplusplus >= 202302L
+struct S;
+// since-cxx23-note at -1 {{forward declaration of 'cwg2547::S'}}
+// since-cxx23-note at -2 {{forward declaration of 'cwg2547::S'}}
+// since-cxx23-note at -3 {{forward declaration of 'cwg2547::S'}}
+bool operator==(S, S) = default; // error: S is not complete
+// since-cxx23-error at -1 {{variable has incomplete type 'S'}}
+// since-cxx23-error at -2 {{variable has incomplete type 'S'}}
+// since-cxx23-error at -3 {{equality comparison operator is not a friend of incomplete class 'cwg2547::S'}}
+struct S {
+ friend bool operator==(S, const S&) = default; // error: parameters of
diff erent types
+ // since-cxx23-error at -1 {{parameters for defaulted equality comparison operator must have the same type (found 'S' vs 'const S &')}}
+};
+enum E { };
+bool operator==(E, E) = default; // error: not a member or friend of a class
+// since-cxx23-error at -1 {{invalid parameter type for non-member defaulted equality comparison operator; found 'E', expected class or reference to a constant class}}
+
+struct S2 {
+ bool operator==(this int, S2) = default;
+ // since-cxx23-error at -1 {{invalid parameter type for defaulted equality comparison operator; found 'int', expected 'const cwg2547::S2 &'}}
+};
+#endif
+} // namespace cwg2547
#if __cplusplus >= 202302L
namespace cwg2553 { // cwg2553: 18 review 2023-07-14
@@ -253,6 +277,41 @@ static_assert(__is_layout_compatible(U, V), "");
#endif
} // namespace cwg2583
+namespace cwg2586 { // cwg2586: 20
+#if __cplusplus >= 202302L
+struct X {
+ X& operator=(this X&, const X&) = default;
+ X& operator=(this X&, X&) = default;
+ X& operator=(this X&&, X&&) = default;
+ // FIXME: The notes could be clearer on *how* the type
diff ers
+ // e.g., "if an explicit object parameter is used it must be of type reference to 'X'"
+ X& operator=(this int, const X&) = default;
+ // since-cxx23-warning at -1 {{explicitly defaulted copy assignment operator is implicitly deleted}}
+ // since-cxx23-note at -2 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
+ X& operator=(this X, const X&) = default;
+ // since-cxx23-warning at -1 {{explicitly defaulted copy assignment operator is implicitly deleted}}
+ // since-cxx23-note at -2 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
+};
+struct Y {
+ void operator=(this int, const Y&); // This is copy constructor, suppresses implicit declaration
+};
+static_assert([]<typename T = Y>{
+ return !requires(T t, const T& ct) { t = ct; };
+}());
+
+struct Z {
+ bool operator==(this const Z&, const Z&) = default;
+ bool operator==(this Z, Z) = default;
+ bool operator==(this Z, const Z&) = default;
+ // since-cxx23-error at -1 {{parameters for defaulted equality comparison operator must have the same type (found 'Z' vs 'const Z &')}}
+ bool operator==(this const Z&, Z) = default;
+ // since-cxx23-error at -1 {{parameters for defaulted equality comparison operator must have the same type (found 'const Z &' vs 'Z')}}
+ bool operator==(this int, Z) = default;
+ // since-cxx23-error at -1 {{invalid parameter type for defaulted equality comparison operator; found 'int', expected 'const cwg2586::Z &'}}
+};
+#endif
+} // namespace cwg2586
+
namespace cwg2598 { // cwg2598: 18
#if __cplusplus >= 201103L
struct NonLiteral {
diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
index 45fee6514c12bc..23fb383fb73cbb 100644
--- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
@@ -729,10 +729,10 @@ struct S2 {
};
S2& S2::operator=(this int&& self, const S2&) = default;
-// expected-error at -1 {{the type of the explicit object parameter of an explicitly-defaulted copy assignment operator should match the type of the class 'S2'}}
+// expected-error at -1 {{the type of the explicit object parameter of an explicitly-defaulted copy assignment operator should be reference to 'S2'}}
S2& S2::operator=(this int&& self, S2&&) = default;
-// expected-error at -1 {{the type of the explicit object parameter of an explicitly-defaulted move assignment operator should match the type of the class 'S2'}}
+// expected-error at -1 {{the type of the explicit object parameter of an explicitly-defaulted move assignment operator should be reference to 'S2'}}
struct Move {
Move& operator=(this int&, Move&&) = default;
@@ -972,3 +972,42 @@ struct R {
f(r_value_ref); // expected-error {{no matching member function for call to 'f'}}
}
};
+
+namespace GH100329 {
+struct A {
+ bool operator == (this const int&, const A&);
+};
+bool A::operator == (this const int&, const A&) = default;
+// expected-error at -1 {{invalid parameter type for defaulted equality comparison operator; found 'const int &', expected 'const GH100329::A &'}}
+} // namespace GH100329
+
+namespace defaulted_assign {
+struct A {
+ A& operator=(this A, const A&) = default;
+ // expected-warning at -1 {{explicitly defaulted copy assignment operator is implicitly deleted}}
+ // expected-note at -2 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
+ A& operator=(this int, const A&) = default;
+ // expected-warning at -1 {{explicitly defaulted copy assignment operator is implicitly deleted}}
+ // expected-note at -2 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
+};
+} // namespace defaulted_assign
+
+namespace defaulted_compare {
+struct A {
+ bool operator==(this A&, const A&) = default;
+ // expected-error at -1 {{defaulted member equality comparison operator must be const-qualified}}
+ bool operator==(this const A, const A&) = default;
+ // expected-error at -1 {{invalid parameter type for defaulted equality comparison operator; found 'const A', expected 'const defaulted_compare::A &'}}
+ bool operator==(this A, A) = default;
+};
+struct B {
+ int a;
+ bool operator==(this B, B) = default;
+};
+static_assert(B{0} == B{0});
+static_assert(B{0} != B{1});
+template<B b>
+struct X;
+static_assert(__is_same(X<B{0}>, X<B{0}>));
+static_assert(!__is_same(X<B{0}>, X<B{1}>));
+} // namespace defaulted_compare
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index 6c283b68aa9656..9b0de55483d275 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -15097,7 +15097,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2547.html">2547</a></td>
<td>DRWP</td>
<td>Defaulted comparison operator function for non-classes</td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 20</td>
</tr>
<tr id="2548">
<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>
<td><a href="https://cplusplus.github.io/CWG/issues/2586.html">2586</a></td>
<td>CD6</td>
<td>Explicit object parameter for assignment and comparison</td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 20</td>
</tr>
<tr class="open" id="2587">
<td><a href="https://cplusplus.github.io/CWG/issues/2587.html">2587</a></td>
More information about the cfe-commits
mailing list