[clang-tidy] Add AnalyzeParameters option to misc-const-correctness #171215
+674
−40
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vbvictor
requested review from
5chmidti,
EugeneZelenko,
HerrCai0907,
localspook and
zeyi2
Member
|
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: Baranov Victor (vbvictor) ChangesCloses #170445. Patch is 33.42 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/171215.diff 9 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
index 8c25e928667df..a6ea6d23dc6ef 100644
--- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
@@ -33,6 +33,14 @@ AST_MATCHER(ReferenceType, isSpelledAsLValue) {
return Node.isSpelledAsLValue();
}
AST_MATCHER(Type, isDependentType) { return Node.isDependentType(); }
+
+AST_MATCHER(FunctionDecl, isTemplate) {
+ return Node.getDescribedFunctionTemplate() != nullptr;
+}
+
+AST_MATCHER(FunctionDecl, isFunctionTemplateSpecialization) {
+ return Node.isFunctionTemplateSpecialization();
+}
} // namespace
ConstCorrectnessCheck::ConstCorrectnessCheck(StringRef Name,
@@ -41,6 +49,7 @@ ConstCorrectnessCheck::ConstCorrectnessCheck(StringRef Name,
AnalyzePointers(Options.get("AnalyzePointers", true)),
AnalyzeReferences(Options.get("AnalyzeReferences", true)),
AnalyzeValues(Options.get("AnalyzeValues", true)),
+ AnalyzeParameters(Options.get("AnalyzeParameters", true)),
WarnPointersAsPointers(Options.get("WarnPointersAsPointers", true)),
WarnPointersAsValues(Options.get("WarnPointersAsValues", false)),
@@ -66,6 +75,7 @@ void ConstCorrectnessCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "AnalyzePointers", AnalyzePointers);
Options.store(Opts, "AnalyzeReferences", AnalyzeReferences);
Options.store(Opts, "AnalyzeValues", AnalyzeValues);
+ Options.store(Opts, "AnalyzeParameters", AnalyzeParameters);
Options.store(Opts, "WarnPointersAsPointers", WarnPointersAsPointers);
Options.store(Opts, "WarnPointersAsValues", WarnPointersAsValues);
@@ -112,15 +122,17 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
const auto FunctionPointerRef =
hasType(hasCanonicalType(referenceType(pointee(functionType()))));
+ const auto CommonExcludeTypes =
+ anyOf(ConstType, ConstReference, RValueReference, TemplateType,
+ FunctionPointerRef, hasType(cxxRecordDecl(isLambda())),
+ AutoTemplateType, isImplicit(), AllowedType);
+
// Match local variables which could be 'const' if not modified later.
// Example: `int i = 10` would match `int i`.
- const auto LocalValDecl = varDecl(
- isLocal(), hasInitializer(anything()),
- unless(anyOf(ConstType, ConstReference, TemplateType,
- hasInitializer(isInstantiationDependent()), AutoTemplateType,
- RValueReference, FunctionPointerRef,
- hasType(cxxRecordDecl(isLambda())), isImplicit(),
- AllowedType)));
+ const auto LocalValDecl =
+ varDecl(isLocal(), hasInitializer(anything()),
+ unless(anyOf(CommonExcludeTypes,
+ hasInitializer(isInstantiationDependent()))));
// Match the function scope for which the analysis of all local variables
// shall be run.
@@ -135,20 +147,83 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
.bind("function-decl");
Finder->addMatcher(FunctionScope, this);
+
+ if (AnalyzeParameters) {
+ const auto ParamMatcher =
+ parmVarDecl(unless(CommonExcludeTypes),
+ anyOf(hasType(referenceType()), hasType(pointerType())))
+ .bind("param-var");
+
+ // Match function parameters which could be 'const' if not modified later.
+ // Example: `void foo(int* ptr)` would match `int* ptr`.
+ const auto FunctionWithParams =
+ functionDecl(
+ hasBody(stmt().bind("scope-params")),
+ has(typeLoc(forEach(ParamMatcher))), unless(cxxMethodDecl()),
+ unless(isFunctionTemplateSpecialization()), unless(isTemplate()))
+ .bind("function-params");
+
+ Finder->addMatcher(FunctionWithParams, this);
+ }
+}
+
+static void addConstFixits(DiagnosticBuilder &Diag, const VarDecl *Variable,
+ const FunctionDecl *Function, ASTContext &Context,
+ Qualifiers::TQ Qualifier,
+ utils::fixit::QualifierTarget Target,
+ utils::fixit::QualifierPolicy Policy) {
+ Diag << addQualifierToVarDecl(*Variable, Context, Qualifier, Target, Policy);
+
+ // If this is a parameter, also add fixits for corresponding parameters in
+ // function declarations
+ if (const auto *ParamDecl = dyn_cast<ParmVarDecl>(Variable)) {
+ const unsigned ParamIdx = ParamDecl->getFunctionScopeIndex();
+
+ for (const FunctionDecl *Redecl : Function->redecls()) {
+ if (Redecl == Function)
+ continue;
+
+ if (ParamIdx < Redecl->getNumParams()) {
+ const ParmVarDecl *RedeclParam = Redecl->getParamDecl(ParamIdx);
+ Diag << addQualifierToVarDecl(*RedeclParam, Context, Qualifier, Target,
+ Policy);
+ }
+ }
+ }
}
/// Classify for a variable in what the Const-Check is interested.
enum class VariableCategory { Value, Reference, Pointer };
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");
+ const Stmt *LocalScope = nullptr;
+ const VarDecl *Variable = nullptr;
+ const FunctionDecl *Function = nullptr;
+ const DeclStmt *VarDeclStmt = nullptr;
+
+ if (const auto *LocalVariable =
+ Result.Nodes.getNodeAs<VarDecl>("local-value")) {
+ Variable = LocalVariable;
+ LocalScope = Result.Nodes.getNodeAs<Stmt>("scope");
+ Function = Result.Nodes.getNodeAs<FunctionDecl>("function-decl");
+ VarDeclStmt = Result.Nodes.getNodeAs<DeclStmt>("decl-stmt");
+ } else if (const auto *Parameter =
+ Result.Nodes.getNodeAs<ParmVarDecl>("param-var")) {
+ Variable = Parameter;
+ LocalScope = Result.Nodes.getNodeAs<Stmt>("scope-params");
+ Function = Result.Nodes.getNodeAs<FunctionDecl>("function-params");
+ } else {
+ llvm_unreachable("didn't match local-value or param-var");
+ }
+
+ assert(Variable && LocalScope && Function);
+
// 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();
+ // be incorrect. Parameters don't need this check - they receive values from
+ // callers
+ const bool CanBeFixIt = isa<ParmVarDecl>(Variable) ||
+ (VarDeclStmt && 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
@@ -172,11 +247,8 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
}
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))
+ if (isMutated(Variable, LocalScope, Function, Result.Context))
return;
auto Diag = diag(Variable->getBeginLoc(),
@@ -187,26 +259,27 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
if (!CanBeFixIt)
return;
using namespace utils::fixit;
+
if (VC == VariableCategory::Value && TransformValues) {
- Diag << addQualifierToVarDecl(*Variable, *Result.Context,
- Qualifiers::Const, QualifierTarget::Value,
- QualifierPolicy::Right);
+ addConstFixits(Diag, Variable, Function, *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;
}
if (VC == VariableCategory::Reference && TransformReferences) {
- Diag << addQualifierToVarDecl(*Variable, *Result.Context,
- Qualifiers::Const, QualifierTarget::Value,
- QualifierPolicy::Right);
+ addConstFixits(Diag, Variable, Function, *Result.Context,
+ Qualifiers::Const, QualifierTarget::Value,
+ QualifierPolicy::Right);
return;
}
if (VC == VariableCategory::Pointer && TransformPointersAsValues) {
- Diag << addQualifierToVarDecl(*Variable, *Result.Context,
- Qualifiers::Const, QualifierTarget::Value,
- QualifierPolicy::Right);
+ addConstFixits(Diag, Variable, Function, *Result.Context,
+ Qualifiers::Const, QualifierTarget::Value,
+ QualifierPolicy::Right);
return;
}
};
@@ -226,9 +299,9 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
return;
using namespace utils::fixit;
if (TransformPointersAsPointers) {
- Diag << addQualifierToVarDecl(*Variable, *Result.Context,
- Qualifiers::Const, QualifierTarget::Pointee,
- QualifierPolicy::Right);
+ addConstFixits(Diag, Variable, Function, *Result.Context,
+ Qualifiers::Const, QualifierTarget::Pointee,
+ QualifierPolicy::Right);
}
};
@@ -271,4 +344,18 @@ void ConstCorrectnessCheck::registerScope(const Stmt *LocalScope,
Analyzer = std::make_unique<ExprMutationAnalyzer>(*LocalScope, *Context);
}
+bool ConstCorrectnessCheck::isMutated(const VarDecl *Variable,
+ const Stmt *Scope,
+ const FunctionDecl *Func,
+ ASTContext *Context) {
+ if (const auto *Param = dyn_cast<ParmVarDecl>(Variable)) {
+ return FunctionParmMutationAnalyzer::getFunctionParmMutationAnalyzer(
+ *Func, *Context, ParamMutationAnalyzerMemoized)
+ ->isMutated(Param);
+ }
+
+ registerScope(Scope, Context);
+ return ScopesCache[Scope]->isMutated(Variable);
+}
+
} // namespace clang::tidy::misc
diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
index fafcac407e029..9f4b2f37e0787 100644
--- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h
@@ -34,13 +34,17 @@ class ConstCorrectnessCheck : public ClangTidyCheck {
private:
void registerScope(const Stmt *LocalScope, ASTContext *Context);
+ bool isMutated(const VarDecl *Variable, const Stmt *Scope,
+ const FunctionDecl *Func, ASTContext *Context);
using MutationAnalyzer = std::unique_ptr<ExprMutationAnalyzer>;
llvm::DenseMap<const Stmt *, MutationAnalyzer> ScopesCache;
llvm::DenseSet<SourceLocation> TemplateDiagnosticsCache;
+ ExprMutationAnalyzer::Memoized ParamMutationAnalyzerMemoized;
const bool AnalyzePointers;
const bool AnalyzeReferences;
const bool AnalyzeValues;
+ const bool AnalyzeParameters;
const bool WarnPointersAsPointers;
const bool WarnPointersAsValues;
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index d1fb1cba3e45a..0154c1adea8cc 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -471,7 +471,8 @@ Changes in existing checks
and avoid false positives of function pointer and fix false
positives on return of non-const pointer and fix false positives on
pointer-to-member operator and avoid false positives when the address
- of a variable is taken to be passed to a function.
+ of a variable is taken to be passed to a function. Added support for
+ analyzing function parameters with the `AnalyzeParameters` option.
- Improved :doc:`misc-coroutine-hostile-raii
<clang-tidy/checks/misc/coroutine-hostile-raii>` check by adding the option
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 01f3b75fc1ffa..dcae96cf860d6 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
@@ -3,9 +3,9 @@
misc-const-correctness
======================
-This check implements detection of local variables which could be declared as
-``const`` but are not. Declaring variables as ``const`` is required or
-recommended by many coding guidelines, such as:
+This check implements detection of local variables and function parameters
+which could be declared as ``const`` but are not. Declaring variables as
+``const`` is required or recommended by many coding guidelines, such as:
`ES.25 <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es25-declare-an-object-const-or-constexpr-unless-you-want-to-modify-its-value-later-on>`_
from the C++ Core Guidelines.
@@ -58,9 +58,9 @@ Limitations
The check does not run on `C` code.
-The check will not analyze templated variables or variables that are
-instantiation dependent. Different instantiations can result in
-different ``const`` correctness properties and in general it is not
+The check will not analyze templated variables and template functions or
+variables that are instantiation dependent. Different instantiations can result
+in different ``const`` correctness properties and in general it is not
possible to find all instantiations of a template. The template might
be used differently in an independent translation unit.
@@ -104,6 +104,20 @@ Options
:option:`WarnPointersAsValues` and :option:`WarnPointersAsPointers`.
Default is `true`.
+.. option:: AnalyzeParameters
+
+ Enable or disable the analysis of function parameters, like
+ ``void foo(int* ptr)``. Only reference and pointer parameters are analyzed.
+ As of current implementation, member function (including constructors) and
+ lambdas are excluded from the analysis. Default is `true`.
+
+ .. code-block:: c++
+
+ // Warning
+ void function(int& param) {}
+ // No warning
+ void function(const int& param) {}
+
.. option:: WarnPointersAsValues
This option enables the suggestion for ``const`` of the pointer itself.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/const-correctness/correctness-fixed.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/const-correctness/correctness-fixed.h
new file mode 100644
index 0000000000000..24f191c448e06
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/const-correctness/correctness-fixed.h
@@ -0,0 +1,11 @@
+struct S {
+ void method() const;
+ int value;
+};
+
+void func_with_ref_param(S const& s);
+
+void func_mixed_params(int x, S const& readonly, S& mutated);
+
+void multi_decl_func(S const& s);
+void multi_decl_func(S const& s);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/const-correctness/correctness.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/const-correctness/correctness.h
new file mode 100644
index 0000000000000..7d1a63c563779
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/const-correctness/correctness.h
@@ -0,0 +1,11 @@
+struct S {
+ void method() const;
+ int value;
+};
+
+void func_with_ref_param(S& s);
+
+void func_mixed_params(int x, S& readonly, S& mutated);
+
+void multi_decl_func(S& s);
+void multi_decl_func(S& s);
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-parameters-header.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-parameters-header.cpp
new file mode 100644
index 0000000000000..bd0d3dbf7068f
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-parameters-header.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp %S/Inputs/const-correctness/correctness.h %t/correctness.h
+// RUN: %check_clang_tidy %s misc-const-correctness %t/temp -- \
+// RUN: -- -I%t -fno-delayed-template-parsing
+// RUN: diff %t/correctness.h %S/Inputs/const-correctness/correctness-fixed.h
+
+#include "correctness.h"
+
+// CHECK-MESSAGES: :[[@LINE+1]]:26: warning: variable 's' of type 'S &' can be declared 'const'
+void func_with_ref_param(S& s) {
+ // CHECK-FIXES: void func_with_ref_param(S const& s) {
+ s.method();
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:31: warning: variable 'readonly' of type 'S &' can be declared 'const'
+void func_mixed_params(int x, S& readonly, S& mutated) {
+ // CHECK-FIXES: void func_mixed_params(int x, S const& readonly, S& mutated) {
+ readonly.method();
+ mutated.value = x;
+}
+
+// CHECK-MESSAGES: :[[@LINE+1]]:22: warning: variable 's' of type 'S &' can be declared 'const'
+void multi_decl_func(S& s) {
+ // CHECK-FIXES: void multi_decl_func(S const& s) {
+ s.method();
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-parameters.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-parameters.cpp
new file mode 100644
index 0000000000000..dbc2f3e25ca95
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-parameters.cpp
@@ -0,0 +1,495 @@
+// RUN: %check_clang_tidy %s misc-const-correctness %t -- \
+// RUN: -config="{CheckOptions: {\
+// RUN: misc-const-correctness.AnalyzeParameters: true, \
+// RUN: misc-const-correctness.WarnPointersAsValues: true, \
+// RUN: misc-const-correctness.TransformPointersAsValues: true \
+// RUN: }}" -- -I %S/Inputs/const-correctness -fno-delayed-template-parsing
+
+struct Bar {
+ void buz() const;
+ void mutate();
+ int value;
+};
+
+struct Foo {
+ void method() const;
+ void mutating_method();
+};
+
+void ref_param_const(Bar& b);
+// CHECK-FIXES: void ref_param_const(Bar const& b);
+
+void ref_param_const(Bar& b) {
+ // CHECK-MESSAGES: [[@LINE-1]]:22: warning: variable 'b' of type 'Bar &' can be declared 'const'
+ // CHECK-FIXES: void ref_param_const(Bar const& b) {
+ b.buz();
+}
+
+void ref_param_already_const(const Foo& f) {
+ f.method();
+}
+
+void ref_param_mutated(Foo& f) {
+ f.mutating_method();
+}
+
+void ref_param_member_modified(Bar& b) {
+ b.value = 42;
+}
+
+void pointer_param_read_only(Bar* b) {
+ // CHECK-MESSAGES: [[@LINE-1]]:30: warning: pointee of variable 'b' of type 'Bar *' can be declared 'const'
+ // CHECK-MESSAGES: [[@LINE-2]]:30: warning: variable 'b' of type 'Bar *' can be declared 'const'
+ // CHECK-FIXES: void pointer_param_read_only(Bar const* const b) {
+ b->buz();
+}
+
+void pointer_param_mutated_pointee(Bar* b) {
+ b->mutate();
+}
+
+void pointer_param_mutated_pointer(Bar* b) {
+ // CHECK-MESSAGES: [[@LINE-1]]:36: warning: pointee of variable 'b' of type 'Bar *' can be declared 'const'
+ // CHECK-FIXES: void pointer_param_mutated_pointer(Bar const* b) {
+ b = nullptr;
+}
+
+void value_param_int(int x) {
+ int y = x + 1;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'y' of type 'int' can be declared 'const'
+ // CHECK-FIXES: int const y = x + 1;
+}
+
+void value_param_struct(Bar b) {
+ b.buz();
+}
+
+void multiple_params_mixed(int x, Bar& b, Foo& f) {
+ // CHECK-MESSAGES: [[@LINE-1]]:35: warning: variable 'b' of type 'Bar &' can be declared 'const'
+ // CHECK-FIXES: void multiple_params_mixed(int x, Bar const& b, Foo& f) {
+ int y = x;
+ // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'y' of type 'int' can be declared 'const'
+ // CHECK-FIXES: int const y = x;
+ b.buz();
+ f.mutating_method();
+}
+
+void rvalue_ref_param(Bar&& b) {
+ b.buz();
+}
+
+void pass_to_const_ref(const Bar& b);
+void pass_to_nonconst_ref(Bar& b);
+
+void param_passed_to_const_ref(Bar& b) {
+ // CHECK-MESSAGES: [[@LINE-1]]:32: warning: variable 'b' of type 'Bar &' can be declared 'const'
+ // CHECK-FIXES: void param_passed_to_const_ref(Bar const& b) {
+ pass_to_const_ref(b);
+}
+
+void param_passed_to_nonconst_ref(Bar& b) ...
[truncated]
|
zeyi2
reviewed
Dec 9, 2025
localspook
reviewed
Dec 9, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #170445.