[PATCH] D31542: [clang-tidy] Extend readability-container-size-empty to add comparisons to newly-constructed objects

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/utils/ASTUtils.cpp:28
+bool IsBinaryOrTernary(const Expr *expr) {
+  if (clang::isa<clang::BinaryOperator>(expr->IgnoreImpCasts()) ||
+      clang::isa<clang::ConditionalOperator>(expr->IgnoreImpCasts())) {
----------------
Rather than call `IgnoreImpCasts()` three times, you should hoist it out of the if statements.


================
Comment at: clang-tidy/utils/ASTUtils.cpp:33
+
+  if (const auto* binary = clang::dyn_cast<clang::CXXOperatorCallExpr>(
+      expr->IgnoreImpCasts())) {
----------------
`binary` doesn't match the usual naming conventions, and the asterisk should bind right rather than left.


================
Comment at: clang-tidy/utils/ASTUtils.h:22
+// Determine whether Expr is a Binary or Ternary expression.
+bool IsBinaryOrTernary(const Expr *expr);
 } // namespace utils
----------------
The parameter name doesn't match our usual naming conventions. I'd just go with `E`.


Repository:
  rL LLVM

https://reviews.llvm.org/D31542