https://github.com/vbvictor updated https://github.com/llvm/llvm-project/pull/171215
>From 3d0b4c1d36266535d108e541b596d700500be9b8 Mon Sep 17 00:00:00 2001 From: Victor Baranov <[email protected]> Date: Tue, 9 Dec 2025 01:15:44 +0300 Subject: [PATCH 1/3] [clang-tidy] Add AnalyzeParameters option to misc-const-correctness --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 145 ++++- .../clang-tidy/misc/ConstCorrectnessCheck.h | 4 + clang-tools-extra/docs/ReleaseNotes.rst | 3 +- .../checks/misc/const-correctness.rst | 26 +- .../const-correctness/correctness-fixed.h | 11 + .../Inputs/const-correctness/correctness.h | 11 + .../const-correctness-parameters-header.cpp | 27 + .../misc/const-correctness-parameters.cpp | 495 ++++++++++++++++++ .../misc/const-correctness-values.cpp | 1 + 9 files changed, 687 insertions(+), 36 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/const-correctness/correctness-fixed.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/const-correctness/correctness.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-parameters-header.cpp create mode 100644 clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-parameters.cpp 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) { + pass_to_nonconst_ref(b); +} + +template<typename T> +void template_param(T& t) { + t.buz(); +} + +void instantiate_template() { + Bar b; + template_param(b); +} + +template<typename T> +void forwarding_ref(T&& t) { + t.method(); +} + +template<typename T> +void specialized_func(T& t) { + t.buz(); +} + +template<> +void specialized_func<Bar>(Bar& b) { + b.buz(); +} + +template<int N> +void non_type_template_param(Bar& b) { + b.buz(); +} + +template<typename... Args> +void variadic_template(Args&... args) { + (args.buz(), ...); +} + +template<typename First, typename... Rest> +void variadic_first_param(First& first, Rest&... rest) { + first.buz(); +} + +template<typename T> +struct is_bar { + static constexpr bool value = false; +}; + +template<> +struct is_bar<Bar> { + static constexpr bool value = true; +}; + +template<typename T, typename = void> +struct enable_if {}; + +template<typename T> +struct enable_if<T, typename T::type> { + using type = typename T::type; +}; + +struct true_type { using type = void; }; +struct false_type {}; + +template<bool B> +struct bool_constant : false_type {}; + +template<> +struct bool_constant<true> : true_type {}; + +template<typename T> +void sfinae_func(T& t, typename enable_if<bool_constant<is_bar<T>::value>>::type* = nullptr) { + t.buz(); +} + +template<typename T> +struct TemplateClass { + void non_template_method(Bar& b) { + b.buz(); + } + + template<typename U> + void template_method(U& u) { + u.buz(); + } + + static void static_method(Bar& b) { + b.buz(); + } +}; + +template struct TemplateClass<int>; + +struct NonTemplateClass { + template<typename T> + void template_method(T& t) { + t.buz(); + } + + template<typename T> + static void static_template_method(T& t) { + t.buz(); + } +}; + +template<typename T = Bar> +void default_template_arg(T& t) { + t.buz(); +} + +template<typename Outer> +struct OuterTemplate { + template<typename Inner> + static void nested_template_func(Outer& o, Inner& i) { + o.buz(); + i.buz(); + } +}; + +template<typename T> +using RefAlias = T&; + +template<typename T> +void alias_template_param(RefAlias<T> t) { + t.buz(); +} + +template<typename T> +void requires_buz(T& t) { +} + +void use_requires_buz() { + Bar b; + requires_buz(b); +} + +void lambda_params() { + auto lambda = [](Bar& b) { + b.buz(); + }; +} + +struct Container { + Container(int* x) {} + void member_func(Bar& b) { + b.buz(); + } +}; + +void builtin_int_ref(int& x) { + // CHECK-MESSAGES: [[@LINE-1]]:22: warning: variable 'x' of type 'int &' can be declared 'const' + // CHECK-FIXES: void builtin_int_ref(int const& x) { + int y = x; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'y' of type 'int' can be declared 'const' + // CHECK-FIXES: int const y = x; +} + +void builtin_int_ref_mutated(int& x) { + x = 42; +} + +void decl_multiple(Bar& b); +// CHECK-FIXES: void decl_multiple(Bar const& b); +void decl_multiple(Bar& b); +// CHECK-FIXES: void decl_multiple(Bar const& b); + +void decl_multiple(Bar& b) { + // CHECK-MESSAGES: [[@LINE-1]]:20: warning: variable 'b' of type 'Bar &' can be declared 'const' + // CHECK-FIXES: void decl_multiple(Bar const& b) { + b.buz(); +} + +void decl_multi_params(int& x, Bar& b, Foo& f); +// CHECK-FIXES: void decl_multi_params(int const& x, Bar const& b, Foo& f); + +void decl_multi_params(int& x, Bar& b, Foo& f) { + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: variable 'x' of type 'int &' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:32: warning: variable 'b' of type 'Bar &' can be declared 'const' + // CHECK-FIXES: void decl_multi_params(int const& 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(); // f is mutated +} + +namespace ns { + void namespaced_func(Foo& f); + // CHECK-FIXES: void namespaced_func(Foo const& f); + + void namespaced_func(Foo& f) { + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: variable 'f' of type 'Foo &' can be declared 'const' + // CHECK-FIXES: void namespaced_func(Foo const& f) { + f.method(); + } +} + +void func_ptr_param(void (*fp)(int&)) { + // CHECK-MESSAGES: [[@LINE-1]]:21: warning: variable 'fp' of type 'void (*)(int &)' can be declared 'const' + // CHECK-FIXES: void func_ptr_param(void (*const fp)(int&)) { + int x = 5; + fp(x); +} + +void func_ptr_param_reassigned(void (*fp)(int&)) { + int x = 5; + fp(x); + fp = nullptr; +} + +struct Block { + void render(int& x) const { x = 0; } +}; + +void member_ptr_param(void (Block::*mp)(int&) const) { +} + +namespace std { + template<typename T> class function; + template<typename R, typename... Args> + class function<R(Args...)> { + public: + template<typename F> function(F&&) {} + R operator()(Args...) const { return R(); } + }; +} + +struct Decl {}; + +void std_function_param(std::function<void(Decl*)> callback) { + Decl d; + callback(&d); + std::function<void(Decl*)> const cb = nullptr; +} + +void std_function_ref_param(std::function<void(int&)> callback) { + int x = 5; + callback(x); +} + +void double_ptr_inner_not_modified(int** pp) { + // CHECK-MESSAGES: [[@LINE-1]]:36: warning: pointee of variable 'pp' of type 'int **' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:36: warning: variable 'pp' of type 'int **' can be declared 'const' + // CHECK-FIXES: void double_ptr_inner_not_modified(int* const* const pp) { + int* p = *pp; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p' of type 'int *' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: variable 'p' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const* const p = *pp; +} + +void double_ptr_inner_modified(int** pp) { + // CHECK-MESSAGES: [[@LINE-1]]:32: warning: variable 'pp' of type 'int **' can be declared 'const' + // CHECK-FIXES: void double_ptr_inner_modified(int** const pp) { + *pp = nullptr; +} + +void double_ptr_outer_not_modified(int** pp) { + // CHECK-MESSAGES: [[@LINE-1]]:36: warning: pointee of variable 'pp' of type 'int **' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:36: warning: variable 'pp' of type 'int **' can be declared 'const' + // CHECK-FIXES: void double_ptr_outer_not_modified(int* const* const pp) { + int* local = *pp; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'local' of type 'int *' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: variable 'local' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const* const local = *pp; +} + +void triple_ptr_read_only(int*** ppp) { + // CHECK-MESSAGES: [[@LINE-1]]:27: warning: pointee of variable 'ppp' of type 'int ***' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:27: warning: variable 'ppp' of type 'int ***' can be declared 'const' + // CHECK-FIXES: void triple_ptr_read_only(int** const* const ppp) { + int** pp = *ppp; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'pp' of type 'int **' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: variable 'pp' of type 'int **' can be declared 'const' + // CHECK-FIXES: int* const* const pp = *ppp; +} + +void triple_ptr_middle_modified(int*** ppp) { + // CHECK-MESSAGES: [[@LINE-1]]:33: warning: pointee of variable 'ppp' of type 'int ***' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:33: warning: variable 'ppp' of type 'int ***' can be declared 'const' + // CHECK-FIXES: void triple_ptr_middle_modified(int** const* const ppp) { + **ppp = nullptr; +} + +void ref_to_ptr_both_readonly(int*& p) { + // CHECK-MESSAGES: [[@LINE-1]]:31: warning: variable 'p' of type 'int *&' can be declared 'const' + // CHECK-FIXES: void ref_to_ptr_both_readonly(int* const& p) { + int val = *p; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'val' of type 'int' can be declared 'const' + // CHECK-FIXES: int const val = *p; +} + +void ref_to_ptr_pointer_modified(int*& p) { + p = nullptr; +} + +void ref_to_ptr_pointee_modified(int*& p) { + // CHECK-MESSAGES: [[@LINE-1]]:34: warning: variable 'p' of type 'int *&' can be declared 'const' + // CHECK-FIXES: void ref_to_ptr_pointee_modified(int* const& p) { + *p = 42; +} + +void ref_to_const_ptr(int* const& p) { + int val = *p; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'val' of type 'int' can be declared 'const' + // CHECK-FIXES: int const val = *p; +} + +void const_ref_to_ptr(int* const& p) { + *p = 42; // Pointee modification is allowed +} + +void ref_to_ptr_to_const(const int*& p) { + // CHECK-MESSAGES: [[@LINE-1]]:26: warning: variable 'p' of type 'const int *&' can be declared 'const' + // CHECK-FIXES: void ref_to_ptr_to_const(const int* const& p) { + int val = *p; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'val' of type 'int' can be declared 'const' + // CHECK-FIXES: int const val = *p; +} + +void ref_to_ptr_to_const_modified(const int*& p) { + p = nullptr; +} + +void double_ptr_const_inner(const int** pp) { + // CHECK-MESSAGES: [[@LINE-1]]:29: warning: pointee of variable 'pp' of type 'const int **' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:29: warning: variable 'pp' of type 'const int **' can be declared 'const' + // CHECK-FIXES: void double_ptr_const_inner(const int* const* const pp) { + const int* p = *pp; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p' of type 'const int *' can be declared 'const' + // CHECK-FIXES: const int* const p = *pp; +} + +void double_ptr_modify_value(int** pp) { + // CHECK-MESSAGES: [[@LINE-1]]:30: warning: pointee of variable 'pp' of type 'int **' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:30: warning: variable 'pp' of type 'int **' can be declared 'const' + // CHECK-FIXES: void double_ptr_modify_value(int* const* const pp) { + **pp = 42; +} + +void ref_to_double_ptr(int**& pp) { + // CHECK-MESSAGES: [[@LINE-1]]:24: warning: variable 'pp' of type 'int **&' can be declared 'const' + // CHECK-FIXES: void ref_to_double_ptr(int** const& pp) { + int* p = *pp; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p' of type 'int *' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: variable 'p' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const* const p = *pp; +} + +void ref_to_double_ptr_outer_modified(int**& pp) { + pp = nullptr; +} + +void ref_to_double_ptr_inner_modified(int**& pp) { + // CHECK-MESSAGES: [[@LINE-1]]:39: warning: variable 'pp' of type 'int **&' can be declared 'const' + // CHECK-FIXES: void ref_to_double_ptr_inner_modified(int** const& pp) { + *pp = nullptr; +} + +void array_of_ptrs(int* arr[]) { + int* p = arr[0]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p' of type 'int *' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: variable 'p' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const* const p = arr[0]; +} + +void ptr_to_array(int (*arr)[10]) { + // CHECK-MESSAGES: [[@LINE-1]]:19: warning: pointee of variable 'arr' of type 'int (*)[10]' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:19: warning: variable 'arr' of type 'int (*)[10]' can be declared 'const' + // CHECK-FIXES: void ptr_to_array(int const(*const arr)[10]) { + int val = (*arr)[0]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'val' of type 'int' can be declared 'const' + // CHECK-FIXES: int const val = (*arr)[0]; +} + +void ptr_to_array_modified(int (*arr)[10]) { + // CHECK-MESSAGES: [[@LINE-1]]:28: warning: variable 'arr' of type 'int (*)[10]' can be declared 'const' + // CHECK-FIXES: void ptr_to_array_modified(int (*const arr)[10]) { + (*arr)[0] = 42; +} + +void ref_to_ptr_array(int* (&arr)[5]) { + // CHECK-MESSAGES: [[@LINE-1]]:23: warning: variable 'arr' of type 'int *(&)[5]' can be declared 'const' + // CHECK-FIXES: void ref_to_ptr_array(int* const(&arr)[5]) { + int* p = arr[0]; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: pointee of variable 'p' of type 'int *' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:3: warning: variable 'p' of type 'int *' can be declared 'const' + // CHECK-FIXES: int const* const p = arr[0]; +} + +void ref_to_ptr_array_modified(int* (&arr)[5]) { + arr[0] = nullptr; +} + +void struct_ptr_param(Bar** bp) { + // CHECK-MESSAGES: [[@LINE-1]]:23: warning: pointee of variable 'bp' of type 'Bar **' can be declared 'const' + // CHECK-MESSAGES: [[@LINE-2]]:23: warning: variable 'bp' of type 'Bar **' can be declared 'const' + // CHECK-FIXES: void struct_ptr_param(Bar* const* const bp) { + (*bp)->buz(); +} + +void struct_ptr_param_modified(Bar** bp) { + // CHECK-MESSAGES: [[@LINE-1]]:32: warning: variable 'bp' of type 'Bar **' can be declared 'const' + // CHECK-FIXES: void struct_ptr_param_modified(Bar** const bp) { + (*bp)->mutate(); +} 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 4cef7f4ce116e..ced07501c01d8 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 @@ -1,5 +1,6 @@ // RUN: %check_clang_tidy %s misc-const-correctness %t -- \ // RUN: -config="{CheckOptions: {\ +// RUN: misc-const-correctness.AnalyzeParameters: false, \ // RUN: misc-const-correctness.TransformValues: true, \ // RUN: misc-const-correctness.WarnPointersAsValues: false, \ // RUN: misc-const-correctness.WarnPointersAsPointers: false, \ >From 0575a7f14e86cc852c2e5b7d9ec5e3f985f516e6 Mon Sep 17 00:00:00 2001 From: Victor Baranov <[email protected]> Date: Wed, 10 Dec 2025 20:26:38 +0300 Subject: [PATCH 2/3] fix review --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 38 ++++++------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp index a6ea6d23dc6ef..d399b2724632d 100644 --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -130,9 +130,8 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { // 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(CommonExcludeTypes, - hasInitializer(isInstantiationDependent())))); + varDecl(isLocal(), hasInitializer(unless(isInstantiationDependent())), + unless(CommonExcludeTypes)); // Match the function scope for which the analysis of all local variables // shall be run. @@ -140,7 +139,7 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { functionDecl( hasBody(stmt(forEachDescendant( declStmt(containsAnyDeclaration( - LocalValDecl.bind("local-value")), + LocalValDecl.bind("value")), unless(has(decompositionDecl()))) .bind("decl-stmt"))) .bind("scope"))) @@ -152,16 +151,16 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { const auto ParamMatcher = parmVarDecl(unless(CommonExcludeTypes), anyOf(hasType(referenceType()), hasType(pointerType()))) - .bind("param-var"); + .bind("value"); // 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")), + hasBody(stmt().bind("scope")), has(typeLoc(forEach(ParamMatcher))), unless(cxxMethodDecl()), unless(isFunctionTemplateSpecialization()), unless(isTemplate())) - .bind("function-params"); + .bind("function-decl"); Finder->addMatcher(FunctionWithParams, this); } @@ -196,32 +195,17 @@ static void addConstFixits(DiagnosticBuilder &Diag, const VarDecl *Variable, enum class VariableCategory { Value, Reference, Pointer }; void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { - 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"); - } + const auto* Variable = Result.Nodes.getNodeAs<VarDecl>("value"); + const auto* LocalScope = Result.Nodes.getNodeAs<Stmt>("scope"); + const auto* Function = Result.Nodes.getNodeAs<FunctionDecl>("function-decl"); + const auto* VarDeclStmt = Result.Nodes.getNodeAs<DeclStmt>("decl-stmt"); 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. Parameters don't need this check - they receive values from - // callers + // callers. const bool CanBeFixIt = isa<ParmVarDecl>(Variable) || (VarDeclStmt && VarDeclStmt->isSingleDecl()); >From 479f7ff6163be555452591f4e1248ec2c5fb6cd3 Mon Sep 17 00:00:00 2001 From: Victor Baranov <[email protected]> Date: Wed, 10 Dec 2025 20:28:02 +0300 Subject: [PATCH 3/3] format --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp index d399b2724632d..79d63a1a56360 100644 --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -136,13 +136,12 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { // Match the function scope for which the analysis of all local variables // shall be run. const auto FunctionScope = - functionDecl( - hasBody(stmt(forEachDescendant( - declStmt(containsAnyDeclaration( - LocalValDecl.bind("value")), - unless(has(decompositionDecl()))) - .bind("decl-stmt"))) - .bind("scope"))) + functionDecl(hasBody(stmt(forEachDescendant( + declStmt(containsAnyDeclaration( + LocalValDecl.bind("value")), + unless(has(decompositionDecl()))) + .bind("decl-stmt"))) + .bind("scope"))) .bind("function-decl"); Finder->addMatcher(FunctionScope, this); @@ -157,9 +156,9 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { // Example: `void foo(int* ptr)` would match `int* ptr`. const auto FunctionWithParams = functionDecl( - hasBody(stmt().bind("scope")), - has(typeLoc(forEach(ParamMatcher))), unless(cxxMethodDecl()), - unless(isFunctionTemplateSpecialization()), unless(isTemplate())) + hasBody(stmt().bind("scope")), has(typeLoc(forEach(ParamMatcher))), + unless(cxxMethodDecl()), unless(isFunctionTemplateSpecialization()), + unless(isTemplate())) .bind("function-decl"); Finder->addMatcher(FunctionWithParams, this); @@ -195,10 +194,10 @@ static void addConstFixits(DiagnosticBuilder &Diag, const VarDecl *Variable, enum class VariableCategory { Value, Reference, Pointer }; void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) { - const auto* Variable = Result.Nodes.getNodeAs<VarDecl>("value"); - const auto* LocalScope = Result.Nodes.getNodeAs<Stmt>("scope"); - const auto* Function = Result.Nodes.getNodeAs<FunctionDecl>("function-decl"); - const auto* VarDeclStmt = Result.Nodes.getNodeAs<DeclStmt>("decl-stmt"); + const auto *LocalScope = Result.Nodes.getNodeAs<Stmt>("scope"); + const auto *Variable = Result.Nodes.getNodeAs<VarDecl>("value"); + const auto *Function = Result.Nodes.getNodeAs<FunctionDecl>("function-decl"); + const auto *VarDeclStmt = Result.Nodes.getNodeAs<DeclStmt>("decl-stmt"); assert(Variable && LocalScope && Function); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
