lebedev.ri added a comment. Nice, i like this! I think the test coverage is good. But what about other `equalsNode()` that you didn't change? Do some of them need this change too?
================ Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:27 +AST_MATCHER_P(Expr, skipCommaOps, + ast_matchers::internal::Matcher<Expr>, InnerMatcher) { ---------------- (Can anyone suggest a better name maybe? I'm not sure this is the most correct name, but i can't suggest a better alternative.) ================ Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:223 cxxOperatorCallExpr(callee(NonConstMethod), hasArgument(0, equalsNode(Exp))), callExpr(callee(expr(anyOf( ---------------- Not changed? ================ Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:225-227 unresolvedMemberExpr(hasObjectExpression(equalsNode(Exp))), cxxDependentScopeMemberExpr( hasObjectExpression(equalsNode(Exp))))))))); ---------------- Can these two happen with comma op? ================ Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:240 castExpr(hasCastKind(CK_ArrayToPointerDecay), unless(hasParent(arraySubscriptExpr())), has(equalsNode(Exp))); // Treat calling `operator->()` of move-only classes as taking address. ---------------- Same, can this happen with comma op? ================ Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:248 returns(nonConstPointerType()))), argumentCountIs(1), hasArgument(0, equalsNode(Exp))); ---------------- And here too ================ Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:265 hasType(templateTypeParmType())))), hasAnyArgument(equalsNode(Exp))), cxxUnresolvedConstructExpr(hasAnyArgument(equalsNode(Exp)))); ---------------- And here ================ Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:279 // adding const.) const auto AsNonConstRefReturn = returnStmt(hasReturnValue(equalsNode(Exp))); ---------------- And here CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58894/new/ https://reviews.llvm.org/D58894 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits