Fix bug 25362 "cppcoreguidelines-pro-bounds-array-to-pointer-decay does not consider const"

[mgehre]

Summary

The current matcher is

implicitCastExpr(unless(hasParent(explicitCastExpr())))

but the AST in the bug is

`-CXXStaticCastExpr 0x2bb64f8 <col:21, col:55> 'void *const *'

static_cast<void *const *> <NoOp>

`-ImplicitCastExpr 0x2bb64e0 <col:47> 'void *const *' <NoOp>
  `-ImplicitCastExpr 0x2bb64c8 <col:47> 'void **'

<ArrayToPointerDecay>

`-DeclRefExpr 0x2bb6458 <col:47> 'void *[2]' lvalue Var

0x2bb59d0 'addrlist' 'void *[2]'
i.e. an ImplicitCastExpr (const cast) between decay and explicit cast.

[aaron.ballman]

I generally think this LGTM, but I would wait for a second confirmation before committing.

clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

33 ↗

(On Diff #39756)

const auto *E, please.

[alexfh]

Looks good with a couple of comments. Thank you for the fix!

clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

33 ↗

(On Diff #39756)

It's not immediately clear what type auto hides here. Please make it the concrete type instead. Same for auto Parents below.

I think, auto should be used, when the initializer contains the concrete type literally, or when the type is difficult to reproduce exactly, or specifying it doesn't add any clearness to the code (e.g. in for (auto I = some_container.begin(); ...) using SomeContainerType<...>::iterator instead of auto doesn't make code easier to read or understand).

test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp

45 ↗

(On Diff #39756)

The function name and the variable name above don't add any value here. I'd shorten them to 1-2 character placeholders.

[mgehre]

Update for review comments

[alexfh]

Still LG.

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.h

public:
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 };
 
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_BOUNDS_ARRAY_TO_POINTER_DECAY_H
-

clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp

 }
 
 AST_MATCHER(Stmt, isInsideOfRangeBeginEndStmt) {
   return stmt(hasAncestor(cxxForRangeStmt(
                   hasRangeBeginEndStmt(hasDescendant(equalsNode(&Node))))))
       .matches(Node, Finder, Builder);
 }
 
+AST_MATCHER_P(Expr, hasParentIgnoringImpCasts,
+              ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+  const Expr *E = &Node;
+  do {
+    ASTContext::DynTypedNodeList Parents =
+        Finder->getASTContext().getParents(*E);
+    if (Parents.size() != 1)
+      return false;
+    E = Parents[0].get<Expr>();
+    if (!E)
+      return false;
+  } while (isa<ImplicitCastExpr>(E));
+
+  return InnerMatcher.matches(*E, Finder, Builder);
+}
+
 void ProBoundsArrayToPointerDecayCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
 
   // The only allowed array to pointer decay
   // 1) just before array subscription
   // 2) inside a range-for over an array
   // 3) if it converts a string literal to a pointer
   Finder->addMatcher(
       implicitCastExpr(unless(hasParent(arraySubscriptExpr())),
-                       unless(hasParent(explicitCastExpr())),
+                       unless(hasParentIgnoringImpCasts(explicitCastExpr())),
                        unless(isInsideOfRangeBeginEndStmt()),
                        unless(hasSourceExpression(stringLiteral())))
           .bind("cast"),
       this);
 }
 
 void ProBoundsArrayToPointerDecayCheck::check(
     const MatchFinder::MatchResult &Result) {

clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp

void f() {
 
   for (auto &e : a) // OK, iteration internally decays array to pointer
     e = 1;
 }
 
 const char *g() {
   return "clang"; // OK, decay string literal to pointer
 }
+
+void f2(void *const *);
+void bug25362() {
+  void *a[2];
+  f2(static_cast<void *const*>(a)); // OK, explicit cast
+}