llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Mital Ashok (MitalAshok) <details> <summary>Changes</summary> Previously, the type of explicit object parameters was not checked at all 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 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) --- Full diff: https://github.com/llvm/llvm-project/pull/100419.diff 7 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+3) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1-1) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+27-22) - (modified) clang/test/CXX/class/class.compare/class.compare.default/p1.cpp (+2) - (modified) clang/test/CXX/drs/cwg25xx.cpp (+55) - (modified) clang/test/SemaCXX/cxx2b-deducing-this.cpp (+10-2) - (modified) clang/www/cxx_dr_status.html (+2-2) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 65de90f69e198..550414ae82fdd 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -149,6 +149,9 @@ Bug Fixes to C++ Support - Fixed a crash when an expression with a dependent ``__typeof__`` type is used as the operand of a unary operator. (#GH97646) +- 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. + Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 810abe4f23e31..d147992bca18d 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -9741,7 +9741,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 04b8d88cae217..273e83748d1fe 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -7617,9 +7617,13 @@ bool Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD, // parameter is of (possibly different) type “reference to C”, // in which case the type of F1 would differ 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 { @@ -8704,7 +8708,8 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD, if (!RD) RD = MD->getParent(); QualType T = MD->getFunctionObjectParameterType(); - if (!T.isConstQualified()) { + if (!T.getNonReferenceType().isConstQualified() && + (MD->isImplicitObjectMemberFunction() || T->isLValueReferenceType())) { SourceLocation Loc, InsertLoc; if (MD->isExplicitObjectMemberFunction()) { Loc = MD->getParamDecl(0)->getBeginLoc(); @@ -8723,11 +8728,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()) { @@ -8754,18 +8765,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(); @@ -8773,14 +8781,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) { @@ -8820,7 +8825,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 ddf82f432c2ea..a195e0548152d 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 0934f0cc19c6a..fa31ffaa2d077 100644 --- a/clang/test/CXX/drs/cwg25xx.cpp +++ b/clang/test/CXX/drs/cwg25xx.cpp @@ -92,6 +92,26 @@ using ::cwg2521::operator""_div; #endif } // namespace cwg2521 +namespace cwg2547 { // cwg2547: 20 +#if __cplusplus >= 202302L +struct S; // since-cxx23-note 3 {{forward declaration of 'cwg2547::S'}} +bool operator==(S, S) = default; // error: S is not complete +// since-cxx23-error@-1 2 {{variable has incomplete type 'S'}} +// since-cxx23-error@-2 {{equality comparison operator is not a friend of incomplete class 'cwg2547::S'}} +struct S { + friend bool operator==(S, const S&) = default; // error: parameters of different types + // since-cxx23-error@-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@-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@-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 @@ -249,6 +269,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 differs + // 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@-1 {{explicitly defaulted copy assignment operator is implicitly deleted}} + // since-cxx23-note@-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@-1 {{explicitly defaulted copy assignment operator is implicitly deleted}} + // since-cxx23-note@-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@-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@-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@-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 4811b6052254c..5f2ab555e44e8 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@-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@-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@-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@-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; @@ -965,3 +965,11 @@ void f(); }; void a::f(this auto) {} // expected-error {{an explicit object parameter cannot appear in a non-member function}} } + +namespace GH100329 { +struct A { + bool operator == (this const int&, const A&); +}; +bool A::operator == (this const int&, const A&) = default; +// expected-error@-1 {{invalid parameter type for defaulted equality comparison operator; found 'const int &', expected 'const GH100329::A &'}} +} // namespace GH100329 diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html index 937f67981e296..e6a44dd122580 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>DR</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> `````````` </details> https://github.com/llvm/llvm-project/pull/100419 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
