[clang-tools-extra] af07d7b - [clang-tidy] Improved cppcoreguidelines-pro-type-const-cast (#69501)

via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 25 22:11:04 PDT 2023


Author: Piotr Zegar
Date: 2023-10-26T07:11:01+02:00
New Revision: af07d7ba883b6e4921820d88b6679f294a0b9fa5

URL: https://github.com/llvm/llvm-project/commit/af07d7ba883b6e4921820d88b6679f294a0b9fa5
DIFF: https://github.com/llvm/llvm-project/commit/af07d7ba883b6e4921820d88b6679f294a0b9fa5.diff

LOG: [clang-tidy] Improved cppcoreguidelines-pro-type-const-cast (#69501)

Improved cppcoreguidelines-pro-type-const-cast check to ignore casts to
const type (controlled by option) and casts in implicitly invoked code.

Fixes #69319

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
    clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst
    clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp
    clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
index ef803ab85fa0841..8c44c1bfb62b6c7 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
@@ -14,13 +14,60 @@ using namespace clang::ast_matchers;
 
 namespace clang::tidy::cppcoreguidelines {
 
+static bool hasConstQualifier(QualType Type) {
+  const QualType PtrType = Type->getPointeeType();
+  if (!PtrType.isNull())
+    return hasConstQualifier(PtrType);
+
+  return Type.isConstQualified();
+}
+
+static bool hasVolatileQualifier(QualType Type) {
+  const QualType PtrType = Type->getPointeeType();
+  if (!PtrType.isNull())
+    return hasVolatileQualifier(PtrType);
+  return Type.isVolatileQualified();
+}
+
+ProTypeConstCastCheck::ProTypeConstCastCheck(StringRef Name,
+                                             ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      StrictMode(Options.getLocalOrGlobal("StrictMode", false)) {}
+
+void ProTypeConstCastCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "StrictMode", StrictMode);
+}
+
 void ProTypeConstCastCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(cxxConstCastExpr().bind("cast"), this);
 }
 
 void ProTypeConstCastCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *MatchedCast = Result.Nodes.getNodeAs<CXXConstCastExpr>("cast");
-  diag(MatchedCast->getOperatorLoc(), "do not use const_cast");
+  if (StrictMode) {
+    diag(MatchedCast->getOperatorLoc(), "do not use const_cast");
+    return;
+  }
+
+  const QualType TargetType = MatchedCast->getType().getCanonicalType();
+  const QualType SourceType =
+      MatchedCast->getSubExpr()->getType().getCanonicalType();
+
+  const bool RemovingConst =
+      hasConstQualifier(SourceType) && !hasConstQualifier(TargetType);
+  const bool RemovingVolatile =
+      hasVolatileQualifier(SourceType) && !hasVolatileQualifier(TargetType);
+
+  if (!RemovingConst && !RemovingVolatile) {
+    // Cast is doing nothing.
+    return;
+  }
+
+  diag(MatchedCast->getOperatorLoc(),
+       "do not use const_cast to remove%select{| const}0%select{| "
+       "and}2%select{| volatile}1 qualifier")
+      << RemovingConst << RemovingVolatile
+      << (RemovingConst && RemovingVolatile);
 }
 
 } // namespace clang::tidy::cppcoreguidelines

diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h
index f7ae9bbb60dcda3..8d93633a321b53f 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h
@@ -13,19 +13,25 @@
 
 namespace clang::tidy::cppcoreguidelines {
 
-/// This check flags all instances of const_cast
+/// Imposes limitations on the use of const_cast within C++ code.
 ///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.html
 class ProTypeConstCastCheck : public ClangTidyCheck {
 public:
-  ProTypeConstCastCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  ProTypeConstCastCheck(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 ac95afd782e1dbe..13003a118c36a51 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -255,6 +255,11 @@ Changes in existing checks
   <clang-tidy/checks/cppcoreguidelines/pro-bounds-constant-array-index>` check
   to perform checks on derived classes of  ``std::array``.
 
+- Improved :doc:`cppcoreguidelines-pro-type-const-cast
+  <clang-tidy/checks/cppcoreguidelines/pro-type-const-cast>` check to ignore
+  casts to ``const`` or ``volatile`` type (controlled by `StrictMode` option)
+  and casts in implicitly invoked code.
+
 - Improved :doc:`cppcoreguidelines-pro-type-member-init
   <clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` check to ignore
   dependent delegate constructors.

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst
index eb572e625f12977..961a591cb81f868 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst
@@ -3,11 +3,35 @@
 cppcoreguidelines-pro-type-const-cast
 =====================================
 
-This check flags all uses of ``const_cast`` in C++ code.
+Imposes limitations on the use of ``const_cast`` within C++ code. It depends on
+the :option:`StrictMode` option setting to determine whether it should flag all
+instances of ``const_cast`` or only those that remove either ``const`` or
+``volatile`` qualifier.
 
-Modifying a variable that was declared const is undefined behavior, even with
-``const_cast``.
+Modifying a variable that has been declared as ``const`` in C++ is generally
+considered undefined behavior, and this remains true even when using
+``const_cast``. In C++, the ``const`` qualifier indicates that a variable is
+intended to be read-only, and the compiler enforces this by disallowing any
+attempts to change the value of that variable.
+
+Removing the ``volatile`` qualifier in C++ can have serious consequences. This
+qualifier indicates that a variable's value can change unpredictably, and
+removing it may lead to undefined behavior, optimization problems, and debugging
+challenges. It's essential to retain the ``volatile`` qualifier in situations
+where the variable's volatility is a crucial aspect of program correctness and
+reliability.
 
 This rule is part of the `Type safety (Type 3)
 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Pro-type-constcast>`_
-profile from the C++ Core Guidelines.
+profile and `ES.50: Don’t cast away const
+<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es50-dont-cast-away-const>`_
+rule from the C++ Core Guidelines.
+
+Options
+-------
+
+.. option:: StrictMode
+
+  When this setting is set to `true`, it means that any usage of ``const_cast``
+  is not allowed. On the other hand, when it's set to `false`, it permits
+  casting to ``const`` or ``volatile`` types. Default value is `false`.

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp
index 2d32e13723abf83..be70e3ba356991a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp
@@ -1,6 +1,86 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-const-cast %t
+// RUN: %check_clang_tidy -check-suffix=STRICT  %s cppcoreguidelines-pro-type-const-cast %t -- -config="{CheckOptions: {StrictMode: true}}"
+// RUN: %check_clang_tidy -check-suffix=NSTRICT %s cppcoreguidelines-pro-type-const-cast %t
 
+namespace Const {
 const int *i;
 int *j;
-void f() { j = const_cast<int *>(i); }
-// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+void f() {
+  j = const_cast<int *>(i);
+  // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:7: warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast]
+  // CHECK-MESSAGES-STRICT:  :[[@LINE-2]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  i = const_cast<const int*>(j);
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  j = *const_cast<int **>(&i);
+  // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast]
+  // CHECK-MESSAGES-STRICT:  :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  i = *const_cast<const int**>(&j);
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  j = &const_cast<int&>(*i);
+  // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast]
+  // CHECK-MESSAGES-STRICT:  :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  i = &const_cast<const int&>(*j);
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+}
+}
+
+namespace Volatile {
+volatile int *i;
+int *j;
+
+void f() {
+  j = const_cast<int *>(i);
+  // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:7: warning: do not use const_cast to remove volatile qualifier [cppcoreguidelines-pro-type-const-cast]
+  // CHECK-MESSAGES-STRICT:  :[[@LINE-2]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  i = const_cast<volatile int*>(j);
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  j = *const_cast<int **>(&i);
+  // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove volatile qualifier [cppcoreguidelines-pro-type-const-cast]
+  // CHECK-MESSAGES-STRICT:  :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  i = *const_cast<volatile int**>(&j);
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  j = &const_cast<int&>(*i);
+  // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove volatile qualifier [cppcoreguidelines-pro-type-const-cast]
+  // CHECK-MESSAGES-STRICT:  :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  i = &const_cast<volatile int&>(*j);
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+}
+}
+
+namespace ConstAndVolatile {
+const volatile int *i;
+int *j;
+
+void f() {
+  j = const_cast<int *>(i);
+  // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:7: warning: do not use const_cast to remove const and volatile qualifier [cppcoreguidelines-pro-type-const-cast]
+  // CHECK-MESSAGES-STRICT:  :[[@LINE-2]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  i = const_cast<const volatile int*>(j);
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  j = *const_cast<int **>(&i);
+  // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove const and volatile qualifier [cppcoreguidelines-pro-type-const-cast]
+  // CHECK-MESSAGES-STRICT:  :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  i = *const_cast<const volatile int**>(&j);
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  j = &const_cast<int&>(*i);
+  // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove const and volatile qualifier [cppcoreguidelines-pro-type-const-cast]
+  // CHECK-MESSAGES-STRICT:  :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+
+  i = &const_cast<const volatile int&>(*j);
+  // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+}
+}

diff  --git a/clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test b/clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test
index 4cb4d1171195a01..9f98d86d0bcc0f1 100644
--- a/clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test
+++ b/clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test
@@ -3,4 +3,4 @@
 const int *i;
 int *j;
 void f() { j = const_cast<int *>(i); }
-// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast]
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast]


        


More information about the cfe-commits mailing list