https://github.com/localspook created https://github.com/llvm/llvm-project/pull/157319
Fixes #60789. Currently, the check will never make `auto` variables `const`. Here's the relevant bit of code: https://github.com/llvm/llvm-project/blob/6b200e21adec0e28407def6fcb2e6c7359fd881b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp#L108-L110 Notice how the matcher's name is `AutoTemplateType`, but it has nothing to do with templates. What it *was* intended to do, I'm not sure, but excluding all `auto` variables can't be right. For lambdas, this is the only justification I can find: https://github.com/llvm/llvm-project/blob/36627e1724504d783dc1cbc466666516d28260e4/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp#L30-L34 Which doesn't convince me. Looking at the test changes, I believe there's only one new false positive, and that one seems to be a symptom of an existing problem that was only being masked by `auto`, but we can absolutely discuss whether the now-diagnosed cases are correct. >From e28e9e234b6165b49884cf254e1fb4efe44fe756 Mon Sep 17 00:00:00 2001 From: Victor Chernyakin <chernyakin.victo...@outlook.com> Date: Sat, 6 Sep 2025 17:35:36 -0700 Subject: [PATCH] [clang-tidy] make `misc-const-correctness` work with `auto` variables and lambdas --- .../clang-tidy/misc/ConstCorrectnessCheck.cpp | 12 ++------ clang-tools-extra/docs/ReleaseNotes.rst | 4 +++ .../const-correctness-pointer-as-values.cpp | 6 +++- .../misc/const-correctness-templates.cpp | 6 ++++ .../const-correctness-transform-values.cpp | 6 ---- .../misc/const-correctness-values.cpp | 28 ++++++++++++++++--- 6 files changed, 42 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp index b32507d66cbac..390046873f2d7 100644 --- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp +++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp @@ -98,17 +98,13 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { hasType(referenceType(pointee(hasCanonicalType(templateTypeParmType())))), hasType(referenceType(pointee(substTemplateTypeParmType())))); - auto AllowedTypeDecl = namedDecl( + const auto AllowedTypeDecl = namedDecl( anyOf(matchers::matchesAnyListedName(AllowedTypes), usingShadowDecl())); const auto AllowedType = hasType(qualType( anyOf(hasDeclaration(AllowedTypeDecl), references(AllowedTypeDecl), pointerType(pointee(hasDeclaration(AllowedTypeDecl)))))); - const auto AutoTemplateType = varDecl( - anyOf(hasType(autoType()), hasType(referenceType(pointee(autoType()))), - hasType(pointerType(pointee(autoType()))))); - const auto FunctionPointerRef = hasType(hasCanonicalType(referenceType(pointee(functionType())))); @@ -117,10 +113,8 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) { const auto LocalValDecl = varDecl( isLocal(), hasInitializer(anything()), unless(anyOf(ConstType, ConstReference, TemplateType, - hasInitializer(isInstantiationDependent()), AutoTemplateType, - RValueReference, FunctionPointerRef, - hasType(cxxRecordDecl(isLambda())), isImplicit(), - AllowedType))); + hasInitializer(isInstantiationDependent()), RValueReference, + FunctionPointerRef, isImplicit(), AllowedType))); // Match the function scope for which the analysis of all local variables // shall be run. diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 780e5b3fc21cf..1ae709c9a2744 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -185,6 +185,10 @@ Changes in existing checks adding an option to allow pointer arithmetic via prefix/postfix increment or decrement operators. +- Improved :doc:`misc-const-correctness + <clang-tidy/checks/misc/const-correctness>` check to diagnose + variables declared with ``auto``. + - Improved :doc:`misc-header-include-cycle <clang-tidy/checks/misc/header-include-cycle>` check performance. 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 74be3dccc9daa..c675482513819 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 @@ -33,7 +33,9 @@ void range_for() { int *p_local2[2] = {nullptr, nullptr}; // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'p_local2' of type 'int *[2]' can be declared 'const' // CHECK-FIXES: int *const p_local2[2] - for (const auto *con_ptr : p_local2) { + for (const auto *p_local3 : p_local2) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'p_local3' of type 'const int *' can be declared 'const' + // CHECK-FIXES: for (const auto *const p_local3 : p_local2) } } @@ -62,6 +64,8 @@ void EmitProtocolMethodList(T &&Methods) { // CHECK-FIXES: SmallVector<const int *> const p_local0 SmallVector<const int *> np_local0; for (const auto *I : Methods) { + // CHECK-MESSAGES: [[@LINE-1]]:8: warning: variable 'I' of type 'const int *' can be declared 'const' + // CHECK-FIXES: for (const auto *const I : Methods) if (I == nullptr) np_local0.push_back(I); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp index 5a890f212a603..323d687cc4f24 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-templates.cpp @@ -10,6 +10,12 @@ template <typename T> void type_dependent_variables() { T value = 42; auto &ref = value; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'ref' of type 'int &' can be declared 'const' + // CHECK-FIXES: auto const&ref = value; + // + // FIXME: This is a false positive, the reference points to a template type + // and needs to be excluded from analysis. See the 'more_template_locals()' + // test in 'const-correctness-values.cpp' for more examples of the problem. T &templateRef = value; int value_int = 42; 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 190d8ecec4c59..46ecb521b74cf 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 @@ -14,12 +14,6 @@ int scoped; float np_scoped = 1; // namespace variables are like globals } // namespace foo -// Lambdas should be ignored, because they do not follow the normal variable -// semantic (e.g. the type is only known to the compiler). -void lambdas() { - auto Lambda = [](int i) { return i < 0; }; -} - void some_function(double, wchar_t); void some_function(double np_arg0, wchar_t np_arg1) { 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 17dcf12e2536c..726caa5ffc531 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 @@ -27,10 +27,19 @@ int np_anonymous_global; int p_anonymous_global = 43; } // namespace -// Lambdas should be ignored, because they do not follow the normal variable -// semantic (e.g. the type is only known to the compiler). void lambdas() { auto Lambda = [](int i) { return i < 0; }; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'Lambda' of type '{{.*}}' can be declared 'const' + // CHECK-FIXES: auto const Lambda + + auto LambdaWithMutableCallOperator = []() mutable {}; + LambdaWithMutableCallOperator(); + + int x = 0; + auto LambdaModifyingCapture = [&x] { ++x; }; + // CHECK-MESSAGES: [[@LINE-1]]:3: warning: variable 'LambdaModifyingCapture' of type '{{.*}}' can be declared 'const' + // CHECK-FIXES: auto const LambdaModifyingCapture + LambdaModifyingCapture(); } void some_function(double, wchar_t); @@ -965,14 +974,23 @@ template <typename T> T *return_ptr() { return &return_ref<T>(); } void auto_usage_variants() { + auto auto_int = int{}; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'auto_int' of type 'int' can be declared 'const' + // CHECK-FIXES: auto const auto_int + auto auto_val0 = int{}; // CHECK-FIXES-NOT: auto const auto_val0 auto &auto_val1 = auto_val0; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'auto_val1' of type 'int &' can be declared 'const' + // CHECK-FIXES: auto const&auto_val1 auto *auto_val2 = &auto_val0; auto auto_ref0 = return_ref<int>(); - // CHECK-FIXES-NOT: auto const auto_ref0 - auto &auto_ref1 = return_ref<int>(); // Bad + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'auto_ref0' of type 'int' can be declared 'const' + // CHECK-FIXES: auto const auto_ref0 + auto &auto_ref1 = return_ref<int>(); + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'auto_ref1' of type 'int &' can be declared 'const' + // CHECK-FIXES: auto const&auto_ref1 auto *auto_ref2 = return_ptr<int>(); auto auto_ptr0 = return_ptr<int>(); @@ -984,6 +1002,8 @@ void auto_usage_variants() { auto auto_td0 = MyTypedef{}; // CHECK-FIXES-NOT: auto const auto_td0 auto &auto_td1 = auto_td0; + // CHECK-MESSAGES:[[@LINE-1]]:3: warning: variable 'auto_td1' of type 'MyTypedef &' (aka 'int &') can be declared 'const' + // CHECK-FIXES: auto const&auto_td1 auto *auto_td2 = &auto_td0; } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
