Skip to content

[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

Conversation

PiotrZSL
Copy link
Member

Add StrictMode option that controls behavior whatever
warnings are emitted for casts on non-polymorphic types.

Fixes: #69414

…-downcast

Add StrictMode option that controls behavior whatever
warnings are emitted for casts on non-polymorphic types.
@llvmbot
Copy link
Member

llvmbot commented Oct 18, 2023

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

Add StrictMode option that controls behavior whatever
warnings are emitted for casts on non-polymorphic types.

Fixes: #69414


Full diff: https://github.com/llvm/llvm-project/pull/69529.diff

5 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.cpp (+20-8)
  • (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeStaticCastDowncastCheck.h (+8-2)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast.rst (+8)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-static-cast-downcast.cpp (+20-19)
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

Copy link
Contributor

@HerrCai0907 HerrCai0907 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@PiotrZSL PiotrZSL merged commit c02b2ab into llvm:main Oct 26, 2023
@PiotrZSL PiotrZSL deleted the 69414-clang-tidy-false-positive-cppcoreguidelines-pro-type-static-cast-downcast-when-base-type-is-not-polymorphic branch October 26, 2023 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[clang-tidy] False positive cppcoreguidelines-pro-type-static-cast-downcast when base type is not polymorphic
3 participants