https://github.com/MitalAshok updated https://github.com/llvm/llvm-project/pull/100419
>From 5d2b3fa876c00869a3964081a57ae23563d18175 Mon Sep 17 00:00:00 2001 From: Mital Ashok <mi...@mitalashok.co.uk> Date: Wed, 24 Jul 2024 16:58:56 +0100 Subject: [PATCH 1/5] [Clang] Check explicit object param for defaulted relational operator has correct type --- clang/docs/ReleaseNotes.rst | 3 + .../clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/lib/Sema/SemaDeclCXX.cpp | 49 +++++++++-------- .../class.compare.default/p1.cpp | 2 + clang/test/CXX/drs/cwg25xx.cpp | 55 +++++++++++++++++++ clang/test/SemaCXX/cxx2b-deducing-this.cpp | 12 +++- clang/www/cxx_dr_status.html | 4 +- 7 files changed, 100 insertions(+), 27 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 65de90f69e1981..550414ae82fdd5 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 810abe4f23e31e..d147992bca18d9 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 04b8d88cae217b..273e83748d1fec 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 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 0934f0cc19c6a9..fa31ffaa2d0778 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 4811b6052254c9..5f2ab555e44e8c 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 937f67981e2963..e6a44dd122580f 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> >From e4bf466d8b8c853d1962379bb9133f79a0bde8c6 Mon Sep 17 00:00:00 2001 From: Mital Ashok <mi...@mitalashok.co.uk> Date: Wed, 24 Jul 2024 19:45:27 +0100 Subject: [PATCH 2/5] Multiple diagnostics --- clang/test/CXX/drs/cwg25xx.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/clang/test/CXX/drs/cwg25xx.cpp b/clang/test/CXX/drs/cwg25xx.cpp index fa31ffaa2d0778..ce53b80ce3aa38 100644 --- a/clang/test/CXX/drs/cwg25xx.cpp +++ b/clang/test/CXX/drs/cwg25xx.cpp @@ -94,10 +94,14 @@ using ::cwg2521::operator""_div; namespace cwg2547 { // cwg2547: 20 #if __cplusplus >= 202302L -struct S; // since-cxx23-note 3 {{forward declaration of 'cwg2547::S'}} +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 2 {{variable has incomplete type 'S'}} -// since-cxx23-error@-2 {{equality comparison operator is not a friend of incomplete class 'cwg2547::S'}} +// 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 different types // since-cxx23-error@-1 {{parameters for defaulted equality comparison operator must have the same type (found 'S' vs 'const S &')}} >From e834fa5c190bbd53568e62ff5b88257ab2bf5ec9 Mon Sep 17 00:00:00 2001 From: Mital Ashok <mi...@mitalashok.co.uk> Date: Wed, 24 Jul 2024 20:09:54 +0100 Subject: [PATCH 3/5] Add tests for defaulted operator= --- clang/docs/ReleaseNotes.rst | 2 ++ clang/test/SemaCXX/cxx2b-deducing-this.cpp | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 550414ae82fdd5..1fa0c9e03698d2 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -152,6 +152,8 @@ Bug Fixes to C++ Support - 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/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp index 5f2ab555e44e8c..83fc1c85faf3f4 100644 --- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp +++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp @@ -973,3 +973,14 @@ struct 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 >From f5eceb850b27a9e1a7d1f8e9b8c02477427e0ad3 Mon Sep 17 00:00:00 2001 From: Mital Ashok <mi...@mitalashok.co.uk> Date: Wed, 24 Jul 2024 20:19:34 +0100 Subject: [PATCH 4/5] Fix handling for `this A&` param missing const in defaulted relational operator --- clang/lib/Sema/SemaDeclCXX.cpp | 2 +- clang/test/SemaCXX/cxx2b-deducing-this.cpp | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 273e83748d1fec..530d3ea081e6c2 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -8707,7 +8707,7 @@ 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(); + QualType T = MD->getFunctionObjectParameterReferenceType(); if (!T.getNonReferenceType().isConstQualified() && (MD->isImplicitObjectMemberFunction() || T->isLValueReferenceType())) { SourceLocation Loc, InsertLoc; diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp index 83fc1c85faf3f4..b60cae65591949 100644 --- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp +++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp @@ -984,3 +984,11 @@ struct A { // 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 A, A) = default; +}; +} // namespace defaulted_compare >From f30d062c9d943c999ac06585e48e253aed976c54 Mon Sep 17 00:00:00 2001 From: Mital Ashok <mi...@mitalashok.co.uk> Date: Thu, 15 Aug 2024 10:48:23 +0100 Subject: [PATCH 5/5] Add more tests for explicit object parameter operator== --- clang/docs/ReleaseNotes.rst | 2 -- clang/test/SemaCXX/cxx2b-deducing-this.cpp | 12 ++++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3575da2ae363c9..0f6612dd77b785 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -256,10 +256,8 @@ 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/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp index 40c40d333a07c3..23fb383fb73cbb 100644 --- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp +++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp @@ -996,6 +996,18 @@ 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits