[PATCH] D132654: [clang-tidy] Fix false positive on `ArrayInitIndexExpr` inside `ProBoundsConstantArrayIndexCheck`
Domján Dániel via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 25 06:37:29 PDT 2022
isuckatcs created this revision.
isuckatcs added reviewers: aaron.ballman, LegalizeAdulthood, njames93.
Herald added subscribers: carlosgalvezp, arphaman, kbarton, xazax.hun, nemanjai.
Herald added a project: All.
isuckatcs requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
There are 3 different cases when an `ArrayInitLoopExpr` is used to initialize an array.
- in the implicit copy/move constructor for a class with an array member
- when a lambda-expression captures an array by value
- when a decomposition declaration decomposes an array
The AST of such expression (usually) looks like this:
|-ArrayInitLoopExpr 'int[3]'
| | |-OpaqueValueExpr 'int[3]' lvalue
| | | `-DeclRefExpr 'int[3]' lvalue Var 'arr' 'int[3]'
| | `-ImplicitCastExpr 'int' <LValueToRValue>
| | `-ArraySubscriptExpr 'int' lvalue
| | |-ImplicitCastExpr 'int *' <ArrayToPointerDecay>
| | | `-OpaqueValueExpr 'int[3]' lvalue
| | | `-DeclRefExpr 'int[3]' lvalue Var 'arr' 'int[3]'
| | `-ArrayInitIndexExpr <<invalid sloc>> 'unsigned long'
We basically always have an `ArraySubscriptExpr`, where the index is an `ArrayInitIndexExpr`.
`ArrayInitIndexExpr` is not a constant, so the checker mentioned in the title reports a warning.
This false positive warning only happens in case of decomposition declaration and lambda capture.
Some example without this patch:
void foo() {
int arr[3];
auto [a, b, c] = arr;
}
---------------------------------------------------------------
warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]
auto [a, b, c] = arr;
^
void foo() {
int arr[3];
[arr]() {};
}
---------------------------------------------------------------
warning: do not use array subscript when the index is not an integer constant expression [cppcoreguidelines-pro-bounds-constant-array-index]
[arr]() {};
^
https://reviews.llvm.org/D132654
Files:
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp
@@ -1,4 +1,5 @@
// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-bounds-constant-array-index -extra-arg=-std=c++17 %t
typedef __SIZE_TYPE__ size_t;
@@ -75,13 +76,34 @@
s[i] = 3; // OK, custom operator
}
+namespace ArrayInitIndexExpr {
struct A {
// The compiler-generated copy constructor uses an ArraySubscriptExpr. Don't warn.
int x[3];
};
-void use_A() {
+void implicitCopyMoveCtor() {
// Force the compiler to generate a copy constructor.
A a;
A a2(a);
+
+ // Force the compiler to generate a move constructor.
+ A a3 = (A&&) a;
+}
+
+void lambdaCapture() {
+ int arr[3];
+
+ // Capturing an array by value uses an ArraySubscriptExpr. Don't warn.
+ [arr](){};
+}
+
+#if __cplusplus >= 201703L
+void structuredBindings() {
+ int arr[3];
+
+ // Creating structured bindings by value uses an ArraySubscriptExpr. Don't warn.
+ auto [a,b,c] = arr;
}
+#endif
+} // namespace ArrayInitIndexExpr
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
@@ -61,6 +61,12 @@
const auto *Matched = Result.Nodes.getNodeAs<Expr>("expr");
const auto *IndexExpr = Result.Nodes.getNodeAs<Expr>("index");
+ // This expression can only appear inside ArrayInitLoopExpr, which
+ // is always implicitly generated. ArrayInitIndexExpr is not a
+ // constant, but we shouldn't report a warning for it.
+ if (isa<ArrayInitIndexExpr>(IndexExpr))
+ return;
+
if (IndexExpr->isValueDependent())
return; // We check in the specialization.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D132654.455562.patch
Type: text/x-patch
Size: 2217 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220825/ad944c48/attachment-0001.bin>
More information about the cfe-commits
mailing list