rsmith added a comment.
If `IncludePossibleEffects` is false, the goal of `HasSideEffect` is to
determine whether we think the user intended for the expression to have a
side-effect. It seems reasonable to assume that any use of an assignment
operator had that intent, so (since it seems like this is not adding false
positives) this seems reasonable to me.
================
Comment at: lib/AST/Expr.cpp:2867-2868
@@ +2866,4 @@
+ // underlaying operator is of assignment type
+ OverloadedOperatorKind binOp =
cast<CXXOperatorCallExpr>(this)->getOperator();
+ if (binOp == OO_Equal || (binOp >= OO_PlusEqual && binOp <= OO_PipeEqual))
{
+ return true;
----------------
Variable names should start with a capital letter.
================
Comment at: lib/AST/Expr.cpp:2868
@@ +2867,3 @@
+ OverloadedOperatorKind binOp =
cast<CXXOperatorCallExpr>(this)->getOperator();
+ if (binOp == OO_Equal || (binOp >= OO_PlusEqual && binOp <= OO_PipeEqual))
{
+ return true;
----------------
Please don't hard-code the order of OO enumerators like this (and this isn't
even correct: you missed `<<=` and `>>=`).
Instead, consider extending `BinaryOperator::isAssignmentOp` /
`BinaryOperator::getOverloadedOpcode` so you can use them for this.
================
Comment at: lib/AST/Expr.cpp:2872
@@ -2864,3 +2871,3 @@
+ }
case CallExprClass:
case CXXMemberCallExprClass:
----------------
Please add a comment indicating the fallthrough here is intentional.
Repository:
rL LLVM
https://reviews.llvm.org/D22910
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
