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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
