[clang] 6a76334 - [Clang] Reject in-class defaulting of previously declared comparison operators
Roy Jacobson via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 17 12:29:58 PST 2023
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&);
More information about the cfe-commits
mailing list