[clang] [Clang] Diagnose defaulted assignment operator with incompatible object parameter (PR #70176)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 26 09:47:00 PDT 2023
https://github.com/cor3ntin updated https://github.com/llvm/llvm-project/pull/70176
>From 9b559ac5504593ab7aa21539b49ece7eea944eb5 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Wed, 25 Oct 2023 10:08:24 +0200
Subject: [PATCH 1/3] [Clang] Diagnose defaulted assignment operator with
incompatible object parameter.
Per https://eel.is/c++draft/dcl.fct.def.default#2.2, the explicit object
parameter of a defaulted special member function must be of the same type
as the one of an equivalent implicitly defaulted function,
ignoring references.
Fixes #69233
---
.../clang/Basic/DiagnosticSemaKinds.td | 3 ++
clang/lib/Sema/SemaDeclCXX.cpp | 19 ++++++++++++
clang/test/SemaCXX/cxx2b-deducing-this.cpp | 31 +++++++++++++++++++
3 files changed, 53 insertions(+)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a673ce726d6c220..6e138683334c2c3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9481,6 +9481,9 @@ def err_defaulted_special_member_return_type : Error<
def err_defaulted_special_member_quals : Error<
"an explicitly-defaulted %select{copy|move}0 assignment operator may not "
"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">;
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 a3f68d4ffc0f6ec..957171b75ba4958 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7748,6 +7748,25 @@ bool Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
HadError = true;
}
}
+ // [C++23][dcl.fct.def.default]/p2.2
+ // if F2 has an implicit object parameter of type “reference to C”,
+ // F1 may be an explicit object member function whose explicit object
+ // 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))) {
+ if (DeleteOnTypeMismatch)
+ ShouldDeleteForTypeMismatch = true;
+ else {
+ Diag(MD->getLocation(),
+ diag::err_defaulted_special_member_explicit_object_mismatch)
+ << (CSM == CXXMoveAssignment) << RD
+ << MD->getSourceRange();
+ HadError = true;
+ }
+ }
}
// Check for parameter type matching.
diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
index 535381e876da9c7..f9e73b41e2c330f 100644
--- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
@@ -585,3 +585,34 @@ class Server : public Thing {
S name_;
};
}
+
+namespace GH69233 {
+struct Base {};
+struct S : Base {
+ int j;
+ S& operator=(this Base& self, const S&) = default;
+ // expected-warning at -1 {{explicitly defaulted copy assignment operator is implicitly deleted}}
+ // expected-note at -2 {{function is implicitly deleted because its declared type does not match the type of an implicit copy assignment operator}}
+ // expected-note at -3 {{explicitly defaulted function was implicitly deleted here}}
+};
+
+struct S2 {
+ S2& operator=(this int&& self, const S2&);
+ S2& operator=(this int&& self, S2&&);
+ operator int();
+};
+
+S2& S2::operator=(this int&& self, const S2&) = default;
+// expected-error at -1 {{the type of the explicit object parameter of an explicitly-defaulted copy assignment operator should match the type of the class 'S2'}}
+
+S2& S2::operator=(this int&& self, S2&&) = default;
+// expected-error at -1 {{the type of the explicit object parameter of an explicitly-defaulted move assignment operator should match the type of the class 'S2'}}
+
+void test() {
+ S s;
+ s = s; // expected-error {{object of type 'S' cannot be assigned because its copy assignment operator is implicitly deleted}}
+ S2 s2;
+ s2 = s2;
+}
+
+}
>From e5f681590232af81edddc764b749ad644e1ef0b4 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Thu, 26 Oct 2023 15:48:15 +0200
Subject: [PATCH 2/3] add source range
---
clang/lib/Sema/SemaDeclCXX.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 957171b75ba4958..beb7e5b177c6e9a 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7762,8 +7762,7 @@ bool Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
else {
Diag(MD->getLocation(),
diag::err_defaulted_special_member_explicit_object_mismatch)
- << (CSM == CXXMoveAssignment) << RD
- << MD->getSourceRange();
+ << (CSM == CXXMoveAssignment) << RD << MD->getSourceRange();
HadError = true;
}
}
>From 927731272162e33faabe19c83abac55854bec628 Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Thu, 26 Oct 2023 18:40:15 +0200
Subject: [PATCH 3/3] [Clang] Warn on deprecated specializations used in system
headers.
When the top of the instantiation stack is in user code.
Thge goal of this PR is to allow deprecation of some char_traits
specializations in libc++ as done in https://reviews.llvm.org/D157058
which was later reverted by
https://github.com/llvm/llvm-project/pull/66153#issuecomment-1719578384
As Clang never emitted the libc++ warnings.
Because Clang likes to eagerly instantiate, we
look for the location of the top of the instantiation stack,
and emit a warning if that location is in user code.
The warning emission is forced by temporarily instruct the diag
engine not to silence warning in system headers,
---
clang/docs/ReleaseNotes.rst | 3 ++
clang/include/clang/Sema/Sema.h | 2 ++
clang/lib/Sema/SemaAvailability.cpp | 23 +++++++++++++++
clang/lib/Sema/SemaTemplate.cpp | 22 +++++++++++++++
...ated-specializations-in-system-headers.cpp | 28 +++++++++++++++++++
5 files changed, 78 insertions(+)
create mode 100644 clang/test/SemaCXX/warn-deprecated-specializations-in-system-headers.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a063733e96cb74c..cdf21d3d0c38179 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -398,6 +398,9 @@ Improvements to Clang's diagnostics
24 | return decltype(fun_ptr)( f_ptr /*comment*/);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+- Clang will warn on deprecated specializations used in system headers when their instantiation
+ is caused by user code.
+
Bug Fixes in This Version
-------------------------
- Fixed an issue where a class template specialization whose declaration is
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1e9752345ffd173..91e1a12cf8df1ec 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -8476,6 +8476,8 @@ class Sema final {
ArrayRef<TemplateArgument> SugaredConverted,
ArrayRef<TemplateArgument> CanonicalConverted, bool &HasDefaultArg);
+ SourceLocation getTopMostPointOfInstantiation(const NamedDecl *) const;
+
/// Specifies the context in which a particular template
/// argument is being checked.
enum CheckTemplateArgumentKind {
diff --git a/clang/lib/Sema/SemaAvailability.cpp b/clang/lib/Sema/SemaAvailability.cpp
index 84c06566387ccbe..846a31a79673096 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -536,6 +536,29 @@ static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K,
}
}
+ // We emit deprecation warning for deprecated specializations
+ // when their instantiation stacks originate outside
+ // of a system header, even if the diagnostics is suppresed at the
+ // point of definition.
+ SourceLocation InstantiationLoc =
+ S.getTopMostPointOfInstantiation(ReferringDecl);
+ bool ShouldAllowWarningInSystemHeader =
+ InstantiationLoc != Loc &&
+ !S.getSourceManager().isInSystemHeader(InstantiationLoc);
+ struct AllowWarningInSystemHeaders {
+ AllowWarningInSystemHeaders(DiagnosticsEngine &E,
+ bool AllowWarningInSystemHeaders)
+ : Engine(E), Prev(E.getSuppressSystemWarnings()) {
+ E.setSuppressSystemWarnings(!AllowWarningInSystemHeaders);
+ }
+ ~AllowWarningInSystemHeaders() { Engine.setSuppressSystemWarnings(Prev); }
+
+ private:
+ DiagnosticsEngine &Engine;
+ bool Prev;
+ } SystemWarningOverrideRAII(S.getDiagnostics(),
+ ShouldAllowWarningInSystemHeader);
+
if (!Message.empty()) {
S.Diag(Loc, diag_message) << ReferringDecl << Message << FixIts;
if (ObjCProperty)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index c2477ec0063e418..61d034e13b0a292 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -11615,3 +11615,25 @@ void Sema::checkSpecializationReachability(SourceLocation Loc,
Sema::AcceptableKind::Reachable)
.check(Spec);
}
+
+/// Returns the top most location responsible for the definition of \p N.
+/// If \p N is a a template specialization, this is the location
+/// of the top of the instantiation stack.
+/// Otherwise, the location of \p N is returned.
+SourceLocation Sema::getTopMostPointOfInstantiation(const NamedDecl *N) const {
+ if (!getLangOpts().CPlusPlus || CodeSynthesisContexts.empty())
+ return N->getLocation();
+ if (auto *FD = dyn_cast<FunctionDecl>(N)) {
+ if (!FD->isFunctionTemplateSpecialization())
+ return FD->getLocation();
+ } else if (!isa<ClassTemplateSpecializationDecl,
+ VarTemplateSpecializationDecl>(N)) {
+ return N->getLocation();
+ }
+ for (const CodeSynthesisContext &CSC : CodeSynthesisContexts) {
+ if (!CSC.isInstantiationRecord() || CSC.PointOfInstantiation.isInvalid())
+ continue;
+ return CSC.PointOfInstantiation;
+ }
+ return N->getLocation();
+}
diff --git a/clang/test/SemaCXX/warn-deprecated-specializations-in-system-headers.cpp b/clang/test/SemaCXX/warn-deprecated-specializations-in-system-headers.cpp
new file mode 100644
index 000000000000000..270e4292bf9a47b
--- /dev/null
+++ b/clang/test/SemaCXX/warn-deprecated-specializations-in-system-headers.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+#ifdef BE_THE_HEADER
+#pragma clang system_header
+
+template <typename T>
+struct traits;
+
+template <>
+struct [[ deprecated]] traits<int> {}; // expected-note {{'traits<int>' has been explicitly marked deprecated here}}
+
+template<typename T, typename Trait = traits<T>> // expected-warning {{'traits<int>' is deprecated}}
+struct basic_string {};
+
+// should not warn, defined and used in system headers
+using __do_what_i_say_not_what_i_do = traits<int> ;
+
+template<typename T, typename Trait = traits<double>>
+struct should_not_warn {};
+
+#else
+#define BE_THE_HEADER
+#include __FILE__
+
+basic_string<int> test1; // expected-note {{in instantiation of default argument for 'basic_string<int>' required here}}
+should_not_warn<int> test2;
+
+#endif
More information about the cfe-commits
mailing list