https://github.com/HerrCai0907 created https://github.com/llvm/llvm-project/pull/130492
None >From 87bda4d0f9c87512f42a600599dd65b80d8e7f60 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Sun, 9 Mar 2025 15:43:37 +0000 Subject: [PATCH] [clang-tidy] support pointee mutation check in misc-const-correctness --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 158 ++++++++++++------ .../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, 209 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 6e412e576e5f9..523dfbfd06138 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 @@ -147,64 +167,92 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { } } - // 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 << Variable->getType(); + 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 << Variable->getType(); + 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 (Variable->getType()->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) { + const QualType VT = Variable->getType(); + 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 951b7f20af4c8..8d369f6b790e4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -126,7 +126,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; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits