Skip to content
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

[clang-tidy] Improved cppcoreguidelines-pro-type-const-cast #69501

Conversation

PiotrZSL
Copy link
Member

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

Fixes #69319

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

llvmbot commented Oct 18, 2023

@llvm/pr-subscribers-clang-tidy

Author: Piotr Zegar (PiotrZSL)

Changes

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

Fixes #69319


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

6 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp (+31-2)
  • (modified) clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h (+9-3)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
  • (modified) clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst (+20-4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp (+25-3)
  • (modified) clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test (+1-1)
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
index ef803ab85fa0841..4661c7d072f9256 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp
@@ -14,13 +14,42 @@ 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();
+}
+
+namespace {
+AST_MATCHER(QualType, hasConst) { return hasConstQualifier(Node); }
+} // namespace
+
+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);
+  if (StrictMode)
+    Finder->addMatcher(cxxConstCastExpr().bind("cast"), this);
+  else
+    Finder->addMatcher(cxxConstCastExpr(unless(hasDestinationType(
+                                            hasCanonicalType(hasConst()))))
+                           .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");
+  diag(MatchedCast->getOperatorLoc(),
+       "do not use const_cast%select{ to cast away const|}0")
+      << StrictMode;
 }
 
 } // 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 3e1fbe091c9ff6a..0a9f6691ff68516 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -236,6 +236,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`` 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..6db8f6a2efd3535 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,27 @@
 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 the ``const`` 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.
 
 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`` types. By default, this setting 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..8f48082b6274fce 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,28 @@
-// 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
 
 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 cast away const [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 cast away const [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 cast away const [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]
+}
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..e6d81cf304bb77a 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 cast away const [cppcoreguidelines-pro-type-const-cast]

@HerrCai0907 HerrCai0907 self-requested a review October 20, 2023 05:13
@PiotrZSL PiotrZSL merged commit af07d7b into llvm:main Oct 26, 2023
4 checks passed
@PiotrZSL PiotrZSL deleted the 69319-clang-tidy-complains-about-const_cast-adding-const-cppcoreguidelines-pro-type-const-cast branch October 26, 2023 05:58
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Oct 26, 2023
Improved cppcoreguidelines-pro-type-const-cast check to ignore casts to
const type (controlled by option) and casts in implicitly invoked code.

Fixes llvm#69319
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: complains about const_cast adding const [cppcoreguidelines-pro-type-const-cast]
3 participants