https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/130494
>From 353f538f425ead9ee10ca6c046a6517b9e157db4 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Sun, 9 Mar 2025 15:43:37 +0000 Subject: [PATCH 1/8] [clang-tidy] support pointee mutation check in misc-const-correctness --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 156 ++++++++++++------ .../clang-tidy/misc/ConstCorrectnessCheck.h | 3 + clang-tools-extra/docs/ReleaseNotes.rst | 3 +- .../checks/misc/const-correctness.rst | 44 +++++ .../const-correctness-pointer-as-pointers.cpp | 50 ++++++ .../const-correctness-transform-values.cpp | 1 + .../const-correctness-values-before-cxx23.cpp | 1 + .../misc/const-correctness-values.cpp | 1 + .../misc/const-correctness-wrong-config.cpp | 7 +- 9 files changed, 207 insertions(+), 59 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers.cpp diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp index dbe59233df699..023c834d5700f 100644 --- 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 index 87dddc4faf781..a2dcaff1a2b61 100644 --- 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 index fa68b6fabd549..bc84f192862db 100644 --- 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 index 2e7e0f3602ab9..1950437fdd41a 100644 --- 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 new file mode 100644 index 0000000000000..ec881da2f7b71 --- /dev/null +++ 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 index 109eddc195558..190d8ecec4c59 100644 --- 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 index 3547ec080911e..af626255d9455 100644 --- 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 index 654deead4efc8..4cf78aeef5bd4 100644 --- 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 index 29e92e52ca9c7..12c09225f61d2 100644 --- 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; >From 121797bd611ec5b414cf57f99e77465259f4023b Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Tue, 11 Mar 2025 23:58:27 +0800 Subject: [PATCH 2/8] Apply suggestions from code review --- clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp index 023c834d5700f..86ad2b2fe8181 100644 --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -56,7 +56,7 @@ ConstCorrectnessCheck::ConstCorrectnessCheck(StringRef Name, AnalyzePointers == false) this->configurationDiag( "The check 'misc-const-correctness' will not " - "perform any analysis because both 'AnalyzeValues', " + "perform any analysis because 'AnalyzeValues', " "'AnalyzeReferences' and 'AnalyzePointers' are false."); } >From d8209fcd30802a9fbe7afb1335b9d3d01d411ddc Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Tue, 11 Mar 2025 23:59:13 +0800 Subject: [PATCH 3/8] Apply suggestions from code review --- .../docs/clang-tidy/checks/misc/const-correctness.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 index 1950437fdd41a..763e8dcac3622 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst @@ -137,7 +137,7 @@ Options This option enables the suggestion for ``const`` of the value pointing. Default is `true`. - Requires 'AnalyzePointers' to be 'true'. + Requires :option:`AnalyzePointers` to be `true`. .. code-block:: c++ @@ -225,10 +225,10 @@ Options .. option:: TransformPointersAsPointers - Provides fixit-hints for pointers if the value it pointing to is not changed. + Provides fix-it hints for pointers if the value it pointing to is not changed. Default is `false`. - Requires 'WarnPointersAsPointers' to be 'true'. + Requires :option:`WarnPointersAsPointers` to be `true`. .. code-block:: c++ >From ba277059bded3990383c3ed08ad8df9d9072980e Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Tue, 11 Mar 2025 16:01:00 +0000 Subject: [PATCH 4/8] fix test --- .../clang-tidy/checkers/misc/const-correctness-wrong-config.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 12c09225f61d2..c0ef160c4562d 100644 --- 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 @@ -5,7 +5,7 @@ // 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', 'AnalyzeReferences' and 'AnalyzePointers' are false. [clang-tidy-config] +// CHECK-MESSAGES: warning: The check 'misc-const-correctness' will not perform any analysis because 'AnalyzeValues', 'AnalyzeReferences' and 'AnalyzePointers' are false. [clang-tidy-config] void g() { int p_local0 = 42; >From d9a50a6f04334ea3782184cbf1d542d2e359f2c7 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Tue, 11 Mar 2025 22:20:36 +0000 Subject: [PATCH 5/8] fix review --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 8 ++++---- .../docs/clang-tidy/checks/misc/const-correctness.rst | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp index 86ad2b2fe8181..d1a03fe703f8c 100644 --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -13,7 +13,6 @@ #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; @@ -157,13 +156,14 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { VariableCategory VC = VariableCategory::Value; const QualType VT = Variable->getType(); - if (VT->isReferenceType()) + if (VT->isReferenceType()) { VC = VariableCategory::Reference; - else if (VT->isPointerType()) + } else if (VT->isPointerType()) { VC = VariableCategory::Pointer; - else if (const auto *ArrayT = dyn_cast<ArrayType>(VT)) + } else if (const auto *ArrayT = dyn_cast<ArrayType>(VT)) { if (ArrayT->getElementType()->isPointerType()) VC = VariableCategory::Pointer; + } auto CheckValue = [&]() { // The scope is only registered if the analysis shall be run. 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 index 763e8dcac3622..6cee7abc0bc87 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst @@ -113,8 +113,8 @@ Options .. option:: AnalyzePointers Enable or disable the analysis of pointers variables, like - ``int *ptr = &i;``. For specific checks, see options - `WarnPointersAsValues` and `WarnPointersAsPointers`. + ``int *ptr = &i;``. For specific checks, see + :option:`WarnPointersAsValues` and :option:`WarnPointersAsPointers`. Default is `true`. .. option:: WarnPointersAsValues >From eaf4260b0ef0d283ccb160ac3081ec267d1601f8 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Tue, 11 Mar 2025 22:37:15 +0000 Subject: [PATCH 6/8] add test; fix bugs --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 13 ++++++++----- .../misc/const-correctness-allowed-types.cpp | 3 ++- ...correctness-pointer-as-pointers-values.cpp | 19 +++++++++++++++++++ .../const-correctness-pointer-as-pointers.cpp | 6 +++--- .../const-correctness-pointer-as-values.cpp | 1 + ...orrectness-transform-pointer-as-values.cpp | 1 + 6 files changed, 34 insertions(+), 9 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers-values.cpp diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp index d1a03fe703f8c..db15d549bfec8 100644 --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -210,9 +210,10 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { 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; + auto Diag = + diag(Variable->getBeginLoc(), + "pointee of variable %0 of type %1 can be declared 'const'") + << Variable << VT; if (IsNormalVariableInTemplate) TemplateDiagnosticsCache.insert(Variable->getBeginLoc()); if (!CanBeFixIt) @@ -241,14 +242,16 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { if (WarnPointersAsValues && !VT.isConstQualified()) CheckValue(); if (WarnPointersAsPointers) { - if (const auto *PT = dyn_cast<PointerType>(VT)) + if (const auto *PT = dyn_cast<PointerType>(VT)) { if (!PT->getPointeeType().isConstQualified()) CheckPointee(); - if (const auto *AT = dyn_cast<ArrayType>(VT)) + } + if (const auto *AT = dyn_cast<ArrayType>(VT)) { if (!AT->getElementType().isConstQualified()) { assert(AT->getElementType()->isPointerType()); CheckPointee(); } + } } return; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-allowed-types.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-allowed-types.cpp index a73b4a08d0a71..1ef36b16a3e4a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-allowed-types.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-allowed-types.cpp @@ -3,7 +3,8 @@ // RUN: misc-const-correctness.AllowedTypes: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$;qualified::Type;::fully::QualifiedType;ConstTemplate', \ // RUN: misc-const-correctness.TransformPointersAsValues: true, \ // RUN: misc-const-correctness.TransformReferences: true, \ -// RUN: misc-const-correctness.WarnPointersAsValues: true } \ +// RUN: misc-const-correctness.WarnPointersAsValues: true, \ +// RUN: misc-const-correctness.WarnPointersAsPointers: false } \ // RUN: }" -- -fno-delayed-template-parsing struct SmartPointer { diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers-values.cpp new file mode 100644 index 0000000000000..0401caaf1a515 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-pointers-values.cpp @@ -0,0 +1,19 @@ +// 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: true,\ +// RUN: misc-const-correctness.WarnPointersAsPointers: true,\ +// RUN: misc-const-correctness.TransformPointersAsValues: true,\ +// 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: pointee of variable 'p_local0' of type 'int *' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: variable 'p_local0' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const*const p_local0 = &a[0]; +} 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 index ec881da2f7b71..796dc3c579b4f 100644 --- 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 @@ -13,7 +13,7 @@ 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-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p_local0' of type 'int *' can be declared 'const' // CHECK-FIXES: int const*p_local0 p_local0 = &a[1]; } @@ -21,7 +21,7 @@ void pointee_to_const() { 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-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p_local0' of type 'int *[1]' can be declared 'const' // CHECK-FIXES: int const*p_local0[1] p_local0[0] = &a[1]; } @@ -30,7 +30,7 @@ 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-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p_local0' of type 'char *' can be declared 'const' // CHECK-FIXES: T const*p_local0 p_local0 = &a[1]; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp index 8d12a76950fef..8cbdffaa801a9 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-pointer-as-values.cpp @@ -2,6 +2,7 @@ // RUN: -config='{CheckOptions: \ // RUN: {misc-const-correctness.AnalyzeValues: true,\ // RUN: misc-const-correctness.WarnPointersAsValues: true,\ +// RUN: misc-const-correctness.WarnPointersAsPointers: false,\ // RUN: misc-const-correctness.TransformPointersAsValues: true}}' \ // RUN: -- -fno-delayed-template-parsing diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-pointer-as-values.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-pointer-as-values.cpp index 7a7d227b3741b..22e565180deaf 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-pointer-as-values.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-transform-pointer-as-values.cpp @@ -2,6 +2,7 @@ // RUN: -config='{CheckOptions: \ // RUN: {misc-const-correctness.AnalyzeValues: true,\ // RUN: misc-const-correctness.WarnPointersAsValues: true, \ +// RUN: misc-const-correctness.WarnPointersAsPointers: false, \ // RUN: misc-const-correctness.TransformPointersAsValues: true}\ // RUN: }' -- -fno-delayed-template-parsing >From 69aa7df2f4998c504d4df310fa8003d007eb9084 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Wed, 12 Mar 2025 09:56:31 +0800 Subject: [PATCH 7/8] alphabetical order config --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 31 +++++++++++-------- .../clang-tidy/misc/ConstCorrectnessCheck.h | 14 +++++---- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp index db15d549bfec8..50e6722badf50 100644 --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -38,17 +38,20 @@ AST_MATCHER(Type, isDependentType) { return Node.isDependentType(); } ConstCorrectnessCheck::ConstCorrectnessCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), - AnalyzeValues(Options.get("AnalyzeValues", true)), - AnalyzeReferences(Options.get("AnalyzeReferences", true)), AnalyzePointers(Options.get("AnalyzePointers", true)), - WarnPointersAsValues(Options.get("WarnPointersAsValues", false)), + AnalyzeReferences(Options.get("AnalyzeReferences", true)), + AnalyzeValues(Options.get("AnalyzeValues", true)), + WarnPointersAsPointers(Options.get("WarnPointersAsPointers", true)), - TransformValues(Options.get("TransformValues", true)), - TransformReferences(Options.get("TransformReferences", true)), - TransformPointersAsValues( - Options.get("TransformPointersAsValues", false)), + WarnPointersAsValues(Options.get("WarnPointersAsValues", false)), + TransformPointersAsPointers( Options.get("TransformPointersAsPointers", true)), + TransformPointersAsValues( + Options.get("TransformPointersAsValues", false)), + TransformReferences(Options.get("TransformReferences", true)), + TransformValues(Options.get("TransformValues", true)), + AllowedTypes( utils::options::parseStringList(Options.get("AllowedTypes", ""))) { if (AnalyzeValues == false && AnalyzeReferences == false && @@ -60,17 +63,19 @@ ConstCorrectnessCheck::ConstCorrectnessCheck(StringRef Name, } 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, "AnalyzeReferences", AnalyzeReferences); + Options.store(Opts, "AnalyzeValues", AnalyzeValues); + Options.store(Opts, "WarnPointersAsPointers", WarnPointersAsPointers); + Options.store(Opts, "WarnPointersAsValues", WarnPointersAsValues); - Options.store(Opts, "TransformValues", TransformValues); - Options.store(Opts, "TransformReferences", TransformReferences); - Options.store(Opts, "TransformPointersAsValues", TransformPointersAsValues); Options.store(Opts, "TransformPointersAsPointers", TransformPointersAsPointers); + Options.store(Opts, "TransformPointersAsValues", TransformPointersAsValues); + Options.store(Opts, "TransformReferences", TransformReferences); + Options.store(Opts, "TransformValues", TransformValues); + Options.store(Opts, "AllowedTypes", utils::options::serializeStringList(AllowedTypes)); } diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h index a2dcaff1a2b61..8af59b7fee294 100644 --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h @@ -38,16 +38,18 @@ class ConstCorrectnessCheck : public ClangTidyCheck { llvm::DenseMap<const Stmt *, MutationAnalyzer> ScopesCache; llvm::DenseSet<SourceLocation> TemplateDiagnosticsCache; - const bool AnalyzeValues; - const bool AnalyzeReferences; const bool AnalyzePointers; - const bool WarnPointersAsValues; + const bool AnalyzeReferences; + const bool AnalyzeValues; + const bool WarnPointersAsPointers; + const bool WarnPointersAsValues; - const bool TransformValues; - const bool TransformReferences; - const bool TransformPointersAsValues; const bool TransformPointersAsPointers; + const bool TransformPointersAsValues; + const bool TransformReferences; + const bool TransformValues; + const std::vector<StringRef> AllowedTypes; }; >From 6df3226b7cde37f54a3a320851b4f8b03b26dc32 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Wed, 12 Mar 2025 14:58:54 +0000 Subject: [PATCH 8/8] clean doc --- .../checks/misc/const-correctness.rst | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) 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 index 6cee7abc0bc87..a142b8ebb6f3f 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst @@ -21,7 +21,7 @@ as potential ``const``. int result = i * i; // Before transformation int const result = i * i; // After transformation -The check can analyze values, pointers and references but not (yet) pointees: +The check can analyze values, pointers and references and pointees: .. code-block:: c++ @@ -39,8 +39,9 @@ The check can analyze values, pointers and references but not (yet) pointees: int const& reference_value = potential_const_int; // After transformation int another_copy = reference_value; - // The similar semantics of pointers are not (yet) analyzed. - int *pointer_variable = &potential_const_int; // _NO_ 'const int *pointer_variable' suggestion. + // The similar semantics of pointers are analyzed. + int *pointer_variable = &potential_const_int; // Before transformation + int const*const pointer_variable = &potential_const_int; // After transformation, both pointer itself and pointee are supported. int last_copy = *pointer_variable; The automatic code transformation is only applied to variables that are declared in single @@ -61,22 +62,6 @@ Different instantiations can result in different ``const`` correctness propertie is not possible to find all instantiations of a template. The template might be used differently in an independent translation unit. -Pointees can not be analyzed for constness yet. The following code shows this limitation. - -.. code-block:: c++ - - // Declare a variable that will not be modified. - int constant_value = 42; - - // Declare a pointer to that variable, that does not modify either, but misses 'const'. - // Could be 'const int *pointer_to_constant = &constant_value;' - int *pointer_to_constant = &constant_value; - - // Usage: - int result = 520 * 120 * (*pointer_to_constant); - -This limitation affects the capability to add ``const`` to methods which is not possible, too. - Options ------- _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits