Author: Roy Jacobson Date: 2023-01-17T22:29:51+02:00 New Revision: 6a763343e29f339cf3a9d282a309589174c74f09
URL: https://github.com/llvm/llvm-project/commit/6a763343e29f339cf3a9d282a309589174c74f09 DIFF: https://github.com/llvm/llvm-project/commit/6a763343e29f339cf3a9d282a309589174c74f09.diff LOG: [Clang] Reject in-class defaulting of previously declared comparison operators Comparison operators are not allowed to be defaulted if they were previously declared outside the class. Pretty low-impact, but it's nice to reject this without a linking error. Fixes https://github.com/llvm/llvm-project/issues/51227. Reviewed By: #clang-language-wg, ChuanqiXu Differential Revision: https://reviews.llvm.org/D141803 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/class/class.compare/class.compare.default/p2.cpp clang/test/CXX/class/class.compare/class.compare.default/p3.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index b19b7859cf9f1..7ddf9b75d5745 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -348,6 +348,8 @@ Bug Fixes - Fix issue that the standard C++ modules importer will call global constructor/destructor for the global varaibles in the importing modules. This fixes `Issue 59765 <https://github.com/llvm/llvm-project/issues/59765>`_ +- Reject in-class defaulting of previosly declared comparison operators. Fixes + `Issue 51227 <https://github.com/llvm/llvm-project/issues/51227>`_. Improvements to Clang's diagnostics ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index de3df4f7dbd04..02a6c2c4214e8 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -9363,6 +9363,9 @@ def err_non_first_default_compare_deletes : Error< "defaulting %select{this %sub{select_defaulted_comparison_kind}1|" "the corresponding implicit 'operator==' for this defaulted 'operator<=>'}0 " "would delete it after its first declaration">; +def err_non_first_default_compare_in_class : Error< + "defaulting this %sub{select_defaulted_comparison_kind}0 " + "is not allowed because it was already declared outside the class">; def note_defaulted_comparison_union : Note< "defaulted %0 is implicitly deleted because " "%2 is a %select{union-like class|union}1 with variant members">; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index ea52b703b563e..cf1242beffe99 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -8734,10 +8734,8 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD, bool First = FD == FD->getCanonicalDecl(); - // If we want to delete the function, then do so; there's nothing else to - // check in that case. - if (Info.Deleted) { - if (!First) { + if (!First) { + if (Info.Deleted) { // C++11 [dcl.fct.def.default]p4: // [For a] user-provided explicitly-defaulted function [...] if such a // function is implicitly defined as deleted, the program is ill-formed. @@ -8751,7 +8749,21 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *FD, .visit(); return true; } + if (isa<CXXRecordDecl>(FD->getLexicalDeclContext())) { + // C++20 [class.compare.default]p1: + // [...] A definition of a comparison operator as defaulted that appears + // in a class shall be the first declaration of that function. + Diag(FD->getLocation(), diag::err_non_first_default_compare_in_class) + << (int)DCK; + Diag(FD->getCanonicalDecl()->getLocation(), + diag::note_previous_declaration); + return true; + } + } + // If we want to delete the function, then do so; there's nothing else to + // check in that case. + if (Info.Deleted) { SetDeclDeleted(FD, FD->getLocation()); if (!inTemplateInstantiation() && !FD->isImplicit()) { Diag(FD->getLocation(), diag::warn_defaulted_comparison_deleted) 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 18b4e271c4bf7..f07b19fefbe74 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 @@ -179,6 +179,12 @@ bool operator==(S4 const &, S4 const &) = default; // expected-error{{not a frie struct S5; // expected-note 3{{forward declaration}} bool operator==(S5, S5) = default; // expected-error{{not a friend}} expected-error 2{{has incomplete type}} +struct S6; +bool operator==(const S6&, const S6&); // expected-note {{previous declaration}} +struct S6 { + friend bool operator==(const S6&, const S6&) = default; // expected-error {{because it was already declared outside}} +}; + enum e {}; bool operator==(e, int) = default; // expected-error{{expected class or reference to a constant class}} diff --git a/clang/test/CXX/class/class.compare/class.compare.default/p2.cpp b/clang/test/CXX/class/class.compare/class.compare.default/p2.cpp index dfa9b8fd99331..95c8f308ab604 100644 --- a/clang/test/CXX/class/class.compare/class.compare.default/p2.cpp +++ b/clang/test/CXX/class/class.compare/class.compare.default/p2.cpp @@ -139,13 +139,13 @@ union E2 { struct F; bool operator==(const F&, const F&); -bool operator!=(const F&, const F&); +bool operator!=(const F&, const F&); // expected-note {{previous declaration}} bool operator<=>(const F&, const F&); -bool operator<(const F&, const F&); +bool operator<(const F&, const F&); // expected-note {{previous declaration}} struct F { union { int a; }; friend bool operator==(const F&, const F&) = default; // expected-error {{defaulting this equality comparison operator would delete it after its first declaration}} expected-note {{implicitly deleted because 'F' is a union-like class}} - friend bool operator!=(const F&, const F&) = default; + friend bool operator!=(const F&, const F&) = default; // expected-error {{because it was already declared outside}} friend bool operator<=>(const F&, const F&) = default; // expected-error {{defaulting this three-way comparison operator would delete it after its first declaration}} expected-note {{implicitly deleted because 'F' is a union-like class}} - friend bool operator<(const F&, const F&) = default; + friend bool operator<(const F&, const F&) = default; // expected-error {{because it was already declared outside}} }; diff --git a/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp b/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp index 81a48a393a068..936ca7c5100a2 100644 --- a/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp +++ b/clang/test/CXX/class/class.compare/class.compare.default/p3.cpp @@ -165,22 +165,22 @@ struct F { // FIXME: This rule creates problems for reordering of declarations; is this // really the right model? struct G; -bool operator==(const G&, const G&); -bool operator!=(const G&, const G&); -std::strong_ordering operator<=>(const G&, const G&); -bool operator<(const G&, const G&); -bool operator<=(const G&, const G&); -bool operator>(const G&, const G&); -bool operator>=(const G&, const G&); +bool operator==(const G&, const G&); // expected-note {{previous declaration}} +bool operator!=(const G&, const G&); // expected-note {{previous declaration}} +std::strong_ordering operator<=>(const G&, const G&); // expected-note {{previous declaration}} +bool operator<(const G&, const G&); // expected-note {{previous declaration}} +bool operator<=(const G&, const G&); // expected-note {{previous declaration}} +bool operator>(const G&, const G&); // expected-note {{previous declaration}} +bool operator>=(const G&, const G&); // expected-note {{previous declaration}} struct G { - friend bool operator==(const G&, const G&) = default; - friend bool operator!=(const G&, const G&) = default; - - friend std::strong_ordering operator<=>(const G&, const G&) = default; - friend bool operator<(const G&, const G&) = default; - friend bool operator<=(const G&, const G&) = default; - friend bool operator>(const G&, const G&) = default; - friend bool operator>=(const G&, const G&) = default; + friend bool operator==(const G&, const G&) = default; // expected-error {{because it was already declared outside}} + friend bool operator!=(const G&, const G&) = default; // expected-error {{because it was already declared outside}} + + friend std::strong_ordering operator<=>(const G&, const G&) = default; // expected-error {{because it was already declared outside}} + friend bool operator<(const G&, const G&) = default; // expected-error {{because it was already declared outside}} + friend bool operator<=(const G&, const G&) = default; // expected-error {{because it was already declared outside}} + friend bool operator>(const G&, const G&) = default; // expected-error {{because it was already declared outside}} + friend bool operator>=(const G&, const G&) = default; // expected-error {{because it was already declared outside}} }; bool operator==(const G&, const G&); bool operator!=(const G&, const G&); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
