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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to