-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang-tidy] Add StrictMode to cppcoreguidelines-pro-type-static-cast-downcast #69529
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
PiotrZSL
merged 1 commit into
llvm:main
from
PiotrZSL:69414-clang-tidy-false-positive-cppcoreguidelines-pro-type-static-cast-downcast-when-base-type-is-not-polymorphic
Oct 26, 2023
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-downcast Add StrictMode option that controls behavior whatever warnings are emitted for casts on non-polymorphic types.
@llvm/pr-subscribers-clang-tidy Author: Piotr Zegar (PiotrZSL) ChangesAdd StrictMode option that controls behavior whatever Fixes: #69414 Full diff: https://github.com/llvm/llvm-project/pull/69529.diff 5 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp
index 5344d465ad1fa71..2a89de27760febe 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp
@@ -14,17 +14,24 @@ using namespace clang::ast_matchers;
namespace clang::tidy::cppcoreguidelines {
+ProTypeStaticCastDowncastCheck::ProTypeStaticCastDowncastCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ StrictMode(Options.getLocalOrGlobal("StrictMode", true)) {}
+
+void ProTypeStaticCastDowncastCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "StrictMode", StrictMode);
+}
+
void ProTypeStaticCastDowncastCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
- cxxStaticCastExpr(unless(isInTemplateInstantiation())).bind("cast"),
- this);
+ cxxStaticCastExpr(hasCastKind(CK_BaseToDerived)).bind("cast"), this);
}
void ProTypeStaticCastDowncastCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *MatchedCast = Result.Nodes.getNodeAs<CXXStaticCastExpr>("cast");
- if (MatchedCast->getCastKind() != CK_BaseToDerived)
- return;
QualType SourceType = MatchedCast->getSubExpr()->getType();
const auto *SourceDecl = SourceType->getPointeeCXXRecordDecl();
@@ -33,15 +40,20 @@ void ProTypeStaticCastDowncastCheck::check(
if (!SourceDecl)
return;
- if (SourceDecl->isPolymorphic())
+ if (SourceDecl->isPolymorphic()) {
diag(MatchedCast->getOperatorLoc(),
"do not use static_cast to downcast from a base to a derived class; "
"use dynamic_cast instead")
<< FixItHint::CreateReplacement(MatchedCast->getOperatorLoc(),
"dynamic_cast");
- else
- diag(MatchedCast->getOperatorLoc(),
- "do not use static_cast to downcast from a base to a derived class");
+ return;
+ }
+
+ if (!StrictMode)
+ return;
+
+ diag(MatchedCast->getOperatorLoc(),
+ "do not use static_cast to downcast from a base to a derived class");
}
} // namespace clang::tidy::cppcoreguidelines
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.h
index 33ec8a071c10810..b9e78a82a39f2da 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.h
@@ -20,13 +20,19 @@ namespace clang::tidy::cppcoreguidelines {
/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast.html
class ProTypeStaticCastDowncastCheck : public ClangTidyCheck {
public:
- ProTypeStaticCastDowncastCheck(StringRef Name, ClangTidyContext *Context)
- : ClangTidyCheck(Name, Context) {}
+ ProTypeStaticCastDowncastCheck(StringRef Name, ClangTidyContext *Context);
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
return LangOpts.CPlusPlus;
}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ std::optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
+
+private:
+ const bool StrictMode;
};
} // namespace clang::tidy::cppcoreguidelines
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 3e1fbe091c9ff6a..9dd25bd7cc61835 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -240,6 +240,11 @@ Changes in existing checks
<clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` check to ignore
dependent delegate constructors.
+- Improved :doc:`cppcoreguidelines-pro-type-static-cast-downcast
+ <clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast>` check to
+ disregard casts on non-polymorphic types when the `StrictMode` option is set
+ to `false`.
+
- Improved :doc:`cppcoreguidelines-pro-type-vararg
<clang-tidy/checks/cppcoreguidelines/pro-type-vararg>` check to ignore
false-positives in unevaluated context (e.g., ``decltype``, ``sizeof``, ...).
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast.rst
index bcb353301f028a3..333e6db2aacec30 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast.rst
@@ -14,3 +14,11 @@ unrelated type ``Z``.
This rule is part of the `Type safety (Type.2)
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Pro-type-downcast>`_
profile from the C++ Core Guidelines.
+
+Options
+-------
+
+.. option:: StrictMode
+
+ When set to `false`, no warnings are emitted for casts on non-polymorphic
+ types. Default is `true`.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-static-cast-downcast.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-static-cast-downcast.cpp
index 7a6c027ec479bab..11179b7d2d19b8f 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-static-cast-downcast.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-static-cast-downcast.cpp
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-static-cast-downcast %t
+// RUN: %check_clang_tidy -check-suffixes=NSTRICT,STRICT %s cppcoreguidelines-pro-type-static-cast-downcast %t
+// RUN: %check_clang_tidy -check-suffix=NSTRICT %s cppcoreguidelines-pro-type-static-cast-downcast %t -- -config="{CheckOptions: {StrictMode: false}}"
class Base {
};
@@ -26,11 +27,11 @@ class PolymorphicMultiDerived : public Base, public PolymorphicBase {
void pointers() {
auto P0 = static_cast<Derived*>(new Base());
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
const Base* B0;
auto PC0 = static_cast<const Derived*>(B0);
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
auto P1 = static_cast<Base*>(new Derived()); // OK, upcast to a public base
auto P2 = static_cast<Base*>(new MultiDerived()); // OK, upcast to a public base
@@ -40,13 +41,13 @@ void pointers() {
void pointers_polymorphic() {
auto PP0 = static_cast<PolymorphicDerived*>(new PolymorphicBase());
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
- // CHECK-FIXES: auto PP0 = dynamic_cast<PolymorphicDerived*>(new PolymorphicBase());
+ // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-FIXES-NSTRICT: auto PP0 = dynamic_cast<PolymorphicDerived*>(new PolymorphicBase());
const PolymorphicBase* B0;
auto PPC0 = static_cast<const PolymorphicDerived*>(B0);
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
- // CHECK-FIXES: auto PPC0 = dynamic_cast<const PolymorphicDerived*>(B0);
+ // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:15: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-FIXES-NSTRICT: auto PPC0 = dynamic_cast<const PolymorphicDerived*>(B0);
auto B1 = static_cast<PolymorphicBase*>(new PolymorphicDerived()); // OK, upcast to a public base
@@ -57,27 +58,27 @@ void pointers_polymorphic() {
void arrays() {
Base ArrayOfBase[10];
auto A0 = static_cast<Derived*>(ArrayOfBase);
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
}
void arrays_polymorphic() {
PolymorphicBase ArrayOfPolymorphicBase[10];
auto AP0 = static_cast<PolymorphicDerived*>(ArrayOfPolymorphicBase);
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
- // CHECK-FIXES: auto AP0 = dynamic_cast<PolymorphicDerived*>(ArrayOfPolymorphicBase);
+ // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
+ // CHECK-FIXES-NSTRICT: auto AP0 = dynamic_cast<PolymorphicDerived*>(ArrayOfPolymorphicBase);
}
void references() {
Base B0;
auto R0 = static_cast<Derived&>(B0);
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
Base& RefToBase = B0;
auto R1 = static_cast<Derived&>(RefToBase);
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
const Base& ConstRefToBase = B0;
auto RC1 = static_cast<const Derived&>(ConstRefToBase);
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
Derived RD1;
@@ -87,18 +88,18 @@ void references() {
void references_polymorphic() {
PolymorphicBase B0;
auto RP0 = static_cast<PolymorphicDerived&>(B0);
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
- // CHECK-FIXES: auto RP0 = dynamic_cast<PolymorphicDerived&>(B0);
+ // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
+ // CHECK-FIXES-NSTRICT: auto RP0 = dynamic_cast<PolymorphicDerived&>(B0);
PolymorphicBase& RefToPolymorphicBase = B0;
auto RP1 = static_cast<PolymorphicDerived&>(RefToPolymorphicBase);
- // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
- // CHECK-FIXES: auto RP1 = dynamic_cast<PolymorphicDerived&>(RefToPolymorphicBase);
+ // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-FIXES-NSTRICT: auto RP1 = dynamic_cast<PolymorphicDerived&>(RefToPolymorphicBase);
const PolymorphicBase& ConstRefToPolymorphicBase = B0;
auto RPC2 = static_cast<const PolymorphicDerived&>(ConstRefToPolymorphicBase);
- // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
- // CHECK-FIXES: auto RPC2 = dynamic_cast<const PolymorphicDerived&>(ConstRefToPolymorphicBase);
+ // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:15: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
+ // CHECK-FIXES-NSTRICT: auto RPC2 = dynamic_cast<const PolymorphicDerived&>(ConstRefToPolymorphicBase);
PolymorphicDerived d1;
auto RP2 = static_cast<PolymorphicBase&>(d1); // OK, upcast to a public base
|
HerrCai0907
reviewed
Oct 26, 2023
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast.rst
Show resolved
Hide resolved
HerrCai0907
approved these changes
Oct 26, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add StrictMode option that controls behavior whatever
warnings are emitted for casts on non-polymorphic types.
Fixes: #69414