[PATCH] D21134: clang-tidy: new check readability-misplaced-array-index

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added a comment.

Is there a reason we don't want this check to be a part of the clang frontend, rather than as a clang-tidy check?


================
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:27-28
@@ +26,4 @@
+          hasRHS(ignoringParenImpCasts(
+              anyOf(stringLiteral(), declRefExpr(hasType(pointsTo(qualType()))),
+                    memberExpr(hasType(pointsTo(qualType())))))))
+          .bind("expr"),
----------------
Can this use `hasType(pointerType())` instead of `hasType(pointsTo(qualType()))` ?

================
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:48
@@ +47,3 @@
+
+  auto D =
+      diag(ArraySubscriptE->getLocStart(),
----------------
Should not use `auto` here because the type is not spelled out in the initialization.

================
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:50
@@ +49,3 @@
+      diag(ArraySubscriptE->getLocStart(),
+           "unusual array index syntax, usually the index is inside the []");
+
----------------
alexfh wrote:
> 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"
Instead of "array index syntax", perhaps "array subscript expression"?

================
Comment at: clang-tidy/readability/MisplacedArrayIndexCheck.cpp:52
@@ +51,3 @@
+
+  // Don't even try to resolve macro or include contraptions. Not worth emitting
+  // a fixit for.
----------------
What does "contraptions" mean in this context?


https://reviews.llvm.org/D21134