[clang-tidy][misc-const-correctness] fix fp when using const array type.

[HerrCai0907]

Fixed: #132931
const array is immutable in C/C++ language design, we don't need to
check constness for it.

[llvmbot]

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Congcong Cai (HerrCai0907)

Changes

Fixed: #132931
const array is immutable in C/C++ language design, we don't need to
check constness for it.

---

Full diff: https://github.com/llvm/llvm-project/pull/133018.diff

3 Files Affected:

• (modified) clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp (+5-1)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+2-1)
• (modified) clang-tools-extra/test/clang-tidy/checkers/misc/const-correctness-values.cpp (+8)

diff --git a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
index 50e6722badf50..13eba246faf56 100644
--- a/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp
@@ -89,6 +89,8 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
   const auto ConstReference = hasType(references(isConstQualified()));
   const auto RValueReference = hasType(
       referenceType(anyOf(rValueReferenceType(), unless(isSpelledAsLValue()))));
+  const auto ConstArrayType =
+      hasType(arrayType(hasElementType(isConstQualified())));
 
   const auto TemplateType = anyOf(
       hasType(hasCanonicalType(templateTypeParmType())),
@@ -115,7 +117,7 @@ void ConstCorrectnessCheck::registerMatchers(MatchFinder *Finder) {
   // Example: `int i = 10` would match `int i`.
   const auto LocalValDecl = varDecl(
       isLocal(), hasInitializer(anything()),
-      unless(anyOf(ConstType, ConstReference, TemplateType,
+      unless(anyOf(ConstType, ConstReference, ConstArrayType, TemplateType,
                    hasInitializer(isInstantiationDependent()), AutoTemplateType,
                    RValueReference, FunctionPointerRef,
                    hasType(cxxRecordDecl(isLambda())), isImplicit(),
@@ -161,6 +163,7 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
 
   VariableCategory VC = VariableCategory::Value;
   const QualType VT = Variable->getType();
+  VT->dump();
   if (VT->isReferenceType()) {
     VC = VariableCategory::Reference;
   } else if (VT->isPointerType()) {
@@ -169,6 +172,7 @@ void ConstCorrectnessCheck::check(const MatchFinder::MatchResult &Result) {
     if (ArrayT->getElementType()->isPointerType())
       VC = VariableCategory::Pointer;
   }
+  llvm::errs() << (int)VC << "\n";
 
   auto CheckValue = [&]() {
     // The scope is only registered if the analysis shall be run.
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index aa85105918ecf..7bbf2190ee262 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -146,7 +146,8 @@ Changes in existing checks
   `AllowedTypes`, that excludes specified types from const-correctness
   checking and fixing false positives when modifying variant by ``operator[]``
   with template in parameters and supporting to check pointee mutation by
-  `AnalyzePointers` option.
+  `AnalyzePointers` option and fixing false positives when using const array
+  type.
 
 - Improved :doc:`misc-redundant-expression
   <clang-tidy/checks/misc/redundant-expression>` check by providing additional
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 4cf78aeef5bd4..a80e1e1af1870 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
@@ -1007,3 +1007,11 @@ template <typename T> void f() {
   x[T{}] = 3;
 }
 } // namespace gh127776_false_positive
+
+namespace gh132931_false_positive {
+using T = const int;
+void valid(int i) {
+  const int arr0[] = {1, 2, 3};
+  T arr1[] = {1, 2, 3};
+}
+} // namespace gh132931_false_positive

[carlosgalvezp]

On line 86 it appears the author intended to handle "const arrays" in the "pointee check". They are indeed handled correctly if one of the options is true (I don't recall which one). Is there a bug in that check that we should fix instead?

[carlosgalvezp]

Alternatively we might want to correct/remove the comment

[HerrCai0907]

I think the comment means check the constness of pointer and array instead of const pointer and const array

[carlosgalvezp]

All the tests (including the FP at hand) pass if you remove unless(arrayType() from line 87 (as well as the part of the comment), so I think it would be preferable to do that instead of creating a ConstArrayType.

[carlosgalvezp]

LGTM, minor comment/question!

[carlosgalvezp]

Shouldn't we get a warning here, since arr1 is not used and can therefore be const?

[carlosgalvezp]

Ah sorry I see now T is const int 🤦‍♂️

[carlosgalvezp]

LGTM!