[clang-tidy] support pointee mutation check in misc-const-correctness

HerrCai0907 · HerrCai0907 · commit 353f538f425e · 2025-03-11T06:54:58.000Z
diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
@@ -13,6 +13,8 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "llvm/Support/Casting.h"
+#include <cassert>
 
 using namespace clang::ast_matchers;
 
@@ -39,34 +41,47 @@ ConstCorrectnessCheck::ConstCorrectnessCheck(StringRef Name,
     : ClangTidyCheck(Name, Context),
       AnalyzeValues(Options.get("AnalyzeValues", true)),
       AnalyzeReferences(Options.get("AnalyzeReferences", true)),
+      AnalyzePointers(Options.get("AnalyzePointers", true)),
       WarnPointersAsValues(Options.get("WarnPointersAsValues", false)),
+      WarnPointersAsPointers(Options.get("WarnPointersAsPointers", true)),
       TransformValues(Options.get("TransformValues", true)),
       TransformReferences(Options.get("TransformReferences", true)),
       TransformPointersAsValues(
           Options.get("TransformPointersAsValues", false)),
+      TransformPointersAsPointers(
+          Options.get("TransformPointersAsPointers", true)),
       AllowedTypes(
           utils::options::parseStringList(Options.get("AllowedTypes", ""))) {
-  if (AnalyzeValues == false && AnalyzeReferences == false)
+  if (AnalyzeValues == false && AnalyzeReferences == false &&
+      AnalyzePointers == false)
     this->configurationDiag(
         "The check 'misc-const-correctness' will not "
-        "perform any analysis because both 'AnalyzeValues' and "
-        "'AnalyzeReferences' are false.");
+        "perform any analysis because both 'AnalyzeValues', "
+        "'AnalyzeReferences' and 'AnalyzePointers' are false.");
 }
 
 void ConstCorrectnessCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "AnalyzeValues", AnalyzeValues);
   Options.store(Opts, "AnalyzeReferences", AnalyzeReferences);
+  Options.store(Opts, "AnalyzePointers", AnalyzePointers);
   Options.store(Opts, "WarnPointersAsValues", WarnPointersAsValues);
+  Options.store(Opts, "WarnPointersAsPointers", WarnPointersAsPointers);
 
   Options.store(Opts, "TransformValues", TransformValues);
   Options.store(Opts, "TransformReferences", TransformReferences);
   Options.store(Opts, "TransformPointersAsValues", TransformPointersAsValues);
+  Options.store(Opts, "TransformPointersAsPointers",
+                TransformPointersAsPointers);
   Options.store(Opts, "AllowedTypes",
                 utils::options::serializeStringList(AllowedTypes));
 }
 
 void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
-  const auto ConstType = hasType(isConstQualified());
+  const auto ConstType = hasType(
+      qualType(isConstQualified(),
+               // pointee check will check the const pointer and const array
+               unless(pointerType()), unless(arrayType())));
+
   const auto ConstReference = hasType(references(isConstQualified()));
   const auto RValueReference = hasType(
       referenceType(anyOf(rValueReferenceType(), unless(isSpelledAsLValue()))));
@@ -124,6 +139,11 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *LocalScope = Result.Nodes.getNodeAs<Stmt>("scope");
   const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("local-value");
   const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("function-decl");
+  const auto *VarDeclStmt = Result.Nodes.getNodeAs<DeclStmt>("decl-stmt");
+  // It can not be guaranteed that the variable is declared isolated,
+  // therefore a transformation might effect the other variables as well and
+  // be incorrect.
+  const bool CanBeFixIt = VarDeclStmt != nullptr && VarDeclStmt->isSingleDecl();
 
   /// If the variable was declared in a template it might be analyzed multiple
   /// times. Only one of those instantiations shall emit a warning. NOTE: This
@@ -145,64 +165,90 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
     if (ArrayT->getElementType()->isPointerType())
       VC = VariableCategory::Pointer;
 
-  // Each variable can only be in one category: Value, Pointer, Reference.
-  // Analysis can be controlled for every category.
-  if (VC == VariableCategory::Reference && !AnalyzeReferences)
-    return;
-
-  if (VC == VariableCategory::Reference &&
-      Variable->getType()->getPointeeType()->isPointerType() &&
-      !WarnPointersAsValues)
-    return;
-
-  if (VC == VariableCategory::Pointer && !WarnPointersAsValues)
-    return;
-
-  if (VC == VariableCategory::Value && !AnalyzeValues)
-    return;
-
-  // The scope is only registered if the analysis shall be run.
-  registerScope(LocalScope, Result.Context);
-
-  // Offload const-analysis to utility function.
-  if (ScopesCache[LocalScope]->isMutated(Variable))
-    return;
-
-  auto Diag = diag(Variable->getBeginLoc(),
-                   "variable %0 of type %1 can be declared 'const'")
-              << Variable << Variable->getType();
-  if (IsNormalVariableInTemplate)
-    TemplateDiagnosticsCache.insert(Variable->getBeginLoc());
+  auto CheckValue = [&]() {
+    // The scope is only registered if the analysis shall be run.
+    registerScope(LocalScope, Result.Context);
+
+    // Offload const-analysis to utility function.
+    if (ScopesCache[LocalScope]->isMutated(Variable))
+      return;
+
+    auto Diag = diag(Variable->getBeginLoc(),
+                     "variable %0 of type %1 can be declared 'const'")
+                << Variable << VT;
+    if (IsNormalVariableInTemplate)
+      TemplateDiagnosticsCache.insert(Variable->getBeginLoc());
+    if (!CanBeFixIt)
+      return;
+    using namespace utils::fixit;
+    if (VC == VariableCategory::Value && TransformValues) {
+      Diag << addQualifierToVarDecl(*Variable, *Result.Context,
+                                    Qualifiers::Const, QualifierTarget::Value,
+                                    QualifierPolicy::Right);
+      // FIXME: Add '{}' for default initialization if no user-defined default
+      // constructor exists and there is no initializer.
+      return;
+    }
 
-  const auto *VarDeclStmt = Result.Nodes.getNodeAs<DeclStmt>("decl-stmt");
+    if (VC == VariableCategory::Reference && TransformReferences) {
+      Diag << addQualifierToVarDecl(*Variable, *Result.Context,
+                                    Qualifiers::Const, QualifierTarget::Value,
+                                    QualifierPolicy::Right);
+      return;
+    }
 
-  // It can not be guaranteed that the variable is declared isolated, therefore
-  // a transformation might effect the other variables as well and be incorrect.
-  if (VarDeclStmt == nullptr || !VarDeclStmt->isSingleDecl())
-    return;
+    if (VC == VariableCategory::Pointer && TransformPointersAsValues) {
+      Diag << addQualifierToVarDecl(*Variable, *Result.Context,
+                                    Qualifiers::Const, QualifierTarget::Value,
+                                    QualifierPolicy::Right);
+      return;
+    }
+  };
+
+  auto CheckPointee = [&]() {
+    assert(VC == VariableCategory::Pointer);
+    registerScope(LocalScope, Result.Context);
+    if (ScopesCache[LocalScope]->isPointeeMutated(Variable))
+      return;
+    auto Diag = diag(Variable->getBeginLoc(),
+                     "variable %0 of type %1 can be declared 'const'")
+                << Variable << VT;
+    if (IsNormalVariableInTemplate)
+      TemplateDiagnosticsCache.insert(Variable->getBeginLoc());
+    if (!CanBeFixIt)
+      return;
+    using namespace utils::fixit;
+    if (TransformPointersAsPointers) {
+      Diag << addQualifierToVarDecl(*Variable, *Result.Context,
+                                    Qualifiers::Const, QualifierTarget::Pointee,
+                                    QualifierPolicy::Right);
+    }
+  };
 
-  using namespace utils::fixit;
-  if (VC == VariableCategory::Value && TransformValues) {
-    Diag << addQualifierToVarDecl(*Variable, *Result.Context, Qualifiers::Const,
-                                  QualifierTarget::Value,
-                                  QualifierPolicy::Right);
-    // FIXME: Add '{}' for default initialization if no user-defined default
-    // constructor exists and there is no initializer.
+  // Each variable can only be in one category: Value, Pointer, Reference.
+  // Analysis can be controlled for every category.
+  if (VC == VariableCategory::Value && AnalyzeValues) {
+    CheckValue();
     return;
   }
-
-  if (VC == VariableCategory::Reference && TransformReferences) {
-    Diag << addQualifierToVarDecl(*Variable, *Result.Context, Qualifiers::Const,
-                                  QualifierTarget::Value,
-                                  QualifierPolicy::Right);
+  if (VC == VariableCategory::Reference && AnalyzeReferences) {
+    if (VT->getPointeeType()->isPointerType() && !WarnPointersAsValues)
+      return;
+    CheckValue();
     return;
   }
-
-  if (VC == VariableCategory::Pointer) {
-    if (WarnPointersAsValues && TransformPointersAsValues) {
-      Diag << addQualifierToVarDecl(*Variable, *Result.Context,
-                                    Qualifiers::Const, QualifierTarget::Value,
-                                    QualifierPolicy::Right);
+  if (VC == VariableCategory::Pointer && AnalyzePointers) {
+    if (WarnPointersAsValues && !VT.isConstQualified())
+      CheckValue();
+    if (WarnPointersAsPointers) {
+      if (const auto *PT = dyn_cast<PointerType>(VT))
+        if (!PT->getPointeeType().isConstQualified())
+          CheckPointee();
+      if (const auto *AT = dyn_cast<ArrayType>(VT))
+        if (!AT->getElementType().isConstQualified()) {
+          assert(AT->getElementType()->isPointerType());
+          CheckPointee();
+        }
     }
     return;
   }
diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
@@ -40,11 +40,14 @@ class ConstCorrectnessCheck : public ClangTidyCheck {
 
   const bool AnalyzeValues;
   const bool AnalyzeReferences;
+  const bool AnalyzePointers;
   const bool WarnPointersAsValues;
+  const bool WarnPointersAsPointers;
 
   const bool TransformValues;
   const bool TransformReferences;
   const bool TransformPointersAsValues;
+  const bool TransformPointersAsPointers;
   const std::vector<StringRef> AllowedTypes;
 };
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -132,7 +132,8 @@ Changes in existing checks
   <clang-tidy/checks/misc/const-correctness>` check by adding the option
   `AllowedTypes`, that excludes specified types from const-correctness
   checking and fixing false positives when modifying variant by ``operator[]``
-  with template in parameters.
+  with template in parameters and supporting to check pointee mutation by
+  `AnalyzePointers` option.
 
 - Improved :doc:`misc-redundant-expression
   <clang-tidy/checks/misc/redundant-expression>` check by providing additional
diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst
@@ -110,6 +110,13 @@ Options
     // No warning
     int const& ref = i;
 
+.. option:: AnalyzePointers
+
+  Enable or disable the analysis of pointers variables, like
+  ``int *ptr = &i;``. For specific checks, see options 
+  `WarnPointersAsValues` and `WarnPointersAsPointers`.
+  Default is `true`.
+
 .. option:: WarnPointersAsValues
 
   This option enables the suggestion for ``const`` of the pointer itself.
@@ -125,6 +132,22 @@ Options
     // No warning
     const int *const pointer_variable = &value;
 
+.. option:: WarnPointersAsPointers
+
+  This option enables the suggestion for ``const`` of the value pointing.
+  Default is `true`.
+
+  Requires 'AnalyzePointers' to be 'true'.
+
+  .. code-block:: c++
+
+    int value = 42;
+
+    // No warning
+    const int *const pointer_variable = &value;
+    // Warning
+    int *const pointer_variable = &value;
+
 .. option:: TransformValues
 
   Provides fixit-hints for value types that automatically add ``const`` if
@@ -200,6 +223,27 @@ Options
     int *changing_pointee = &value;
     changing_pointee = &result;
 
+.. option:: TransformPointersAsPointers
+
+  Provides fixit-hints for pointers if the value it pointing to is not changed.
+  Default is `false`.
+
+  Requires 'WarnPointersAsPointers' to be 'true'.
+
+  .. code-block:: c++
+
+    int value = 42;
+
+    // Before
+    int * pointer_variable = &value;
+    // After
+    const int * pointer_variable = &value;
+
+    // Before
+    int * a[] = {&value, &value};
+    // After
+    const int * a[] = {&value, &value};
+
 .. option:: AllowedTypes
 
   A semicolon-separated list of names of types that will be excluded from
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp
@@ -0,0 +1,50 @@
+// RUN: %check_clang_tidy %s misc-const-correctness %t \
+// RUN: -config='{CheckOptions: {\
+// RUN:   misc-const-correctness.AnalyzeValues: false,\
+// RUN:   misc-const-correctness.AnalyzeReferences: false,\
+// RUN:   misc-const-correctness.AnalyzePointers: true,\
+// RUN:   misc-const-correctness.WarnPointersAsValues: false,\
+// RUN:   misc-const-correctness.WarnPointersAsPointers: true,\
+// RUN:   misc-const-correctness.TransformPointersAsValues: false,\
+// RUN:   misc-const-correctness.TransformPointersAsPointers: true\
+// RUN: }}' \
+// RUN: -- -fno-delayed-template-parsing
+
+void pointee_to_const() {
+  int a[] = {1, 2};
+  int *p_local0 = &a[0];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *' can be declared 'const'
+  // CHECK-FIXES: int  const*p_local0
+  p_local0 = &a[1];
+}
+
+void array_of_pointer_to_const() {
+  int a[] = {1, 2};
+  int *p_local0[1] = {&a[0]};
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'int *[1]' can be declared 'const'
+  // CHECK-FIXES: int  const*p_local0[1]
+  p_local0[0] = &a[1];
+}
+
+template<class T>
+void template_fn() {
+  T a[] = {1, 2};
+  T *p_local0 = &a[0];
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local0' of type 'char *' can be declared 'const'
+  // CHECK-FIXES: T  const*p_local0
+  p_local0 = &a[1];
+}
+
+void instantiate() {
+  template_fn<char>();
+  template_fn<int>();
+  template_fn<char const>();
+}
+
+using const_int = int const;
+void ignore_const_alias() {
+  const_int a[] = {1, 2};
+  const_int *p_local0 = &a[0];
+  p_local0 = &a[1];
+}
+
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-values.cpp
@@ -2,6 +2,7 @@
 // RUN:   -config="{CheckOptions: {\
 // RUN:   misc-const-correctness.TransformValues: true,\
 // RUN:   misc-const-correctness.WarnPointersAsValues: false, \
+// RUN:   misc-const-correctness.WarnPointersAsPointers: false, \
 // RUN:   misc-const-correctness.TransformPointersAsValues: false} \
 // RUN:   }" -- -fno-delayed-template-parsing
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values-before-cxx23.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values-before-cxx23.cpp
@@ -2,6 +2,7 @@
 // RUN:   -config="{CheckOptions: {\
 // RUN:     misc-const-correctness.TransformValues: true, \
 // RUN:     misc-const-correctness.WarnPointersAsValues: false, \
+// RUN:     misc-const-correctness.WarnPointersAsPointers: false, \
 // RUN:     misc-const-correctness.TransformPointersAsValues: false \
 // RUN:   }}" -- -fno-delayed-template-parsing
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp
@@ -2,6 +2,7 @@
 // RUN:   -config="{CheckOptions: {\
 // RUN:     misc-const-correctness.TransformValues: true, \
 // RUN:     misc-const-correctness.WarnPointersAsValues: false, \
+// RUN:     misc-const-correctness.WarnPointersAsPointers: false, \
 // RUN:     misc-const-correctness.TransformPointersAsValues: false \
 // RUN:   }}" -- -fno-delayed-template-parsing -fexceptions
 
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-wrong-config.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-wrong-config.cpp
@@ -1,10 +1,11 @@
 // RUN: %check_clang_tidy %s misc-const-correctness %t \
 // RUN: -config='{CheckOptions: \
 // RUN:  {"misc-const-correctness.AnalyzeValues": false,\
-// RUN:   "misc-const-correctness.AnalyzeReferences": false}\
-// RUN:  }' -- -fno-delayed-template-parsing
+// RUN:   "misc-const-correctness.AnalyzeReferences": false,\
+// RUN:   "misc-const-correctness.AnalyzePointers": false\
+// RUN:  }}' -- -fno-delayed-template-parsing
 
-// CHECK-MESSAGES: warning: The check 'misc-const-correctness' will not perform any analysis because both 'AnalyzeValues' and 'AnalyzeReferences' are false. [clang-tidy-config]
+// CHECK-MESSAGES: warning: The check 'misc-const-correctness' will not perform any analysis because both 'AnalyzeValues', 'AnalyzeReferences' and 'AnalyzePointers' are false. [clang-tidy-config]
 
 void g() {
   int p_local0 = 42;
