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.
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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
