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