aaron.ballman added a comment. I noticed a few more things, but mostly nitpicky at this point.
================ Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:22 @@ +21,3 @@ +bool needsParens(const Expr *E) { + E = E->IgnoreImpCasts(); + if (isa<BinaryOperator>(E) || isa<ConditionalOperator>(E)) ---------------- Should we also ignore parens for when people do crazy things, like sizeof((((some_string+other_string))))? ================ Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:25 @@ +24,3 @@ + return true; + if (const auto *Op = dyn_cast<CXXOperatorCallExpr>(E)) { + return Op->getNumArgs() == 2 && Op->getOperator() != OO_Call && ---------------- I missed this earlier, but what about other 2-arg overloaded operators, like placement operator new & operator delete, or operator delete with size (and array forms)? ================ Comment at: clang-tidy/misc/SizeofContainerCheck.cpp:38 @@ +37,3 @@ + sizeOfExpr(has(expr(hasType(hasCanonicalType(hasDeclaration( + recordDecl(matchesName("^(::std::|::string)"), + hasMethod(methodDecl(hasName("size"), isPublic(), ---------------- Why is "|::string" needed? http://reviews.llvm.org/D12759 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits