[PATCH] D14517: Fix bug 25362 "cppcoreguidelines-pro-bounds-array-to-pointer-decay does not consider const"

[Alexander Kornienko via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

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


================
Comment at: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:33
@@ +32,3 @@
+              ast_matchers::internal::Matcher<Expr>, InnerMatcher) {
+  auto E = &Node;
+  do {
----------------
aaron.ballman wrote:
> const auto *E, please.
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). 

================
Comment at: test/clang-tidy/cppcoreguidelines-pro-bounds-array-to-pointer-decay.cpp:45
@@ +44,3 @@
+  void *addrlist[2];
+  void backtrace_symbols(void *const *buffer);
+  backtrace_symbols(static_cast<void *const*>(addrlist)); // OK, explicit cast
----------------
The function name and the variable name above don't add any value here. I'd shorten them to 1-2 character placeholders.


http://reviews.llvm.org/D14517