alexfh added a comment.
Thanks, that's better. Still a couple of comments.
================
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:50
@@ +49,3 @@
+ diag(ArraySubscriptE->getLocStart(),
+ "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) ||
+
!Result.SourceManager->isWrittenInSameFile(ArraySubscriptE->getLocStart(),
----------------
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.
https://reviews.llvm.org/D21134
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
