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@-1 {{forward declaration of 'cwg2547::S'}} +// since-cxx23-note@-2 {{forward declaration of 'cwg2547::S'}} +// since-cxx23-note@-3 {{forward declaration of 'cwg2547::S'}} +bool operator==(S, S) = default; // error: S is not complete +// since-cxx23-error@-1 {{variable has incomplete type 'S'}} +// since-cxx23-error@-2 {{variable has incomplete type 'S'}} +// since-cxx23-error@-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@-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 @@ -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@-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 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@-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; @@ -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@-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@-1 {{explicitly defaulted copy assignment operator is implicitly deleted}} + // expected-note@-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@-1 {{explicitly defaulted copy assignment operator is implicitly deleted}} + // expected-note@-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@-1 {{defaulted member equality comparison operator must be const-qualified}} + bool operator==(this const A, const A&) = default; + // expected-error@-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> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
