[PATCH] D113863: [clang-tidy] Make `readability-container-data-pointer` more robust

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:22
+constexpr llvm::StringLiteral DerefContainerExprName = "deref-container-expr";
+constexpr llvm::StringLiteral AddrofContainerExprName = "addrof-container-expr";
+constexpr llvm::StringLiteral AddressOfName = "address-of";
----------------
Would you mind using `addr-of-container-expr` and renaming this to `AddrOfContainerExprName`?


================
Comment at: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:76
+                    arraySubscriptExpr(hasLHS(ContainerExpr), hasRHS(Zero))))))
+          .bind(AddressOfName),
       this);
----------------
Nice, I do like this, it is quite a bit more succinct.  One thing that is a bit less clear to me is that the previous expression was a bit more restrictive in the parameter types to certain expressions, is there a reason that you don't expect that to matter much in practice?  If a malformed input is provided, we could now match certain things that we didn't previously, or am I not matching up the conditions carefully enough?


================
Comment at: clang-tools-extra/clang-tidy/readability/ContainerDataPointerCheck.cpp:99
+      Lexer::getSourceText(CharSourceRange::getTokenRange(SourceRange),
+                           *Result.SourceManager, getLangOpts()));
+
----------------
The diff shows up odd here, is there a hard tab or is that just the rendering?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113863/new/

https://reviews.llvm.org/D113863