[clang-tidy] Fix false positive on `ArrayInitIndexExpr` inside `ProBoundsConstantArrayIndexCheck`

[isuckatcs]

Summary

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]() {};
 ^

[gribozavr2]

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp

2

You shouldn't need to add an extra RUN line, check_clang_tidy already loops over all language standards.

[isuckatcs]

Removed the unnecessary extra RUN command.

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp

Finder->addMatcher(
       this);
 }
 
 void ProBoundsConstantArrayIndexCheck::check(
     const MatchFinder::MatchResult &Result) {
   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.
 
   Optional<llvm::APSInt> Index =
       IndexExpr->getIntegerConstantExpr(*Result.Context);
   if (!Index) {
     SourceRange BaseRange;
     if (const auto *ArraySubscriptE = dyn_cast<ArraySubscriptExpr>(Matched))

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-bounds-constant-array-index.cpp

 // 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;
 
 namespace std {
   template<typename T, size_t N>
   struct array {
     T& operator[](size_t n);
     T& at(size_t n);

 };
 
 void customOperator() {
   S s;
   int i = 0;
   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