[PATCH] D21134: clang-tidy: new check readability-misplaced-array-index
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 8 06:54:33 PDT 2016
alexfh added a comment.
Thanks, that's better. Still a couple of comments.
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:50
@@ +49,3 @@
+ "unusual array index syntax, usually the index is inside the ");
I'd say "confusing" instead of "unusual". This would also help avoiding the repetition ("unusual ..., usually ..."):
"confusing array index syntax; usually, the index is inside the brackets"
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:54
@@ +53,3 @@
+ // a fixit for.
+ if (hasMacroID(ArraySubscriptE) ||
What exactly is the recursive `hasMacroID` function trying to prevent? Do you have a test that breaks if you just check that the start location of the expression is not a macro?
In most cases, it's enough to check that the whole range is on the same macro expansion level using `Lexer::makeFileCharRange` in order to prevent most problems with macros, while allowing fixes in safe cases, e.g. replacements in parameters of `EXPECT_*` macros from gtest.
More information about the cfe-commits