[PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

aaron.ballman added a comment. In https://reviews.llvm.org/D22910#601083, @Abpostelnicu wrote: > Can someone please show me an example on how to write a test for this patch? You could test this with anything that diagnoses side effects in an unevaluated context (look for

[PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu added a comment. Can someone please show me an example on how to write a test for this patch? https://reviews.llvm.org/D22910 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu updated this revision to Diff 73766. Abpostelnicu marked 3 inline comments as done. https://reviews.llvm.org/D22910 Files: lib/AST/Expr.cpp Index: lib/AST/Expr.cpp === --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@

[PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

aaron.ballman added a comment. This patch is missing tests for the new functionality. > aaron.ballman wrote in ExprCXX.h:109 > Missing the full stop at the end of the sentence. It looks like this comment has not been handled yet. > ExprCXX.h:120 > + // Check to see if a given overloaded

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu updated this revision to Diff 72806. Abpostelnicu marked an inline comment as done. Abpostelnicu added a comment. corrected typo https://reviews.llvm.org/D22910 Files: include/clang/AST/ExprCXX.h lib/AST/Expr.cpp Index: lib/AST/Expr.cpp

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

malcolm.parsons added a subscriber: malcolm.parsons. Comment at: lib/AST/Expr.cpp:2868 @@ +2867,3 @@ +// When looking for potential side-effects, we assume that these +// operators: assignment, increement and decrement are intended +// to have a side-effect and other

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu updated this revision to Diff 72790. Abpostelnicu marked 2 inline comments as done. Abpostelnicu added a comment. i will add the unit tests in the next patch. https://reviews.llvm.org/D22910 Files: include/clang/AST/ExprCXX.h lib/AST/Expr.cpp Index: lib/AST/Expr.cpp

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

aaron.ballman added a comment. In https://reviews.llvm.org/D22910#549611, @sylvestre.ledru wrote: > What about landing this version now (with test) and do the other operators in > the a second commit? thanks I don't see a whole lot of value in that, but I may be missing information -- is

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

On 21 Sep 2016 6:10 am, "Aaron Ballman" wrote: aaron.ballman added a comment. The other thing this patch is missing are tests, btw. Comment at: lib/AST/Expr.cpp:2869 @@ +2868,3 @@ +// assignment operator is intended to have a side-effect and other

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

aaron.ballman added a comment. The other thing this patch is missing are tests, btw. Comment at: lib/AST/Expr.cpp:2869 @@ +2868,3 @@ +// assignment operator is intended to have a side-effect and other overloaded +// operators are not. Otherwise fall through the logic

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu updated this revision to Diff 72026. https://reviews.llvm.org/D22910 Files: include/clang/AST/ExprCXX.h lib/AST/Expr.cpp Index: lib/AST/Expr.cpp === --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -2863,8 +2863,20 @@

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu added inline comments. Comment at: lib/AST/Expr.cpp:2869 @@ +2868,3 @@ +OverloadedOperatorKind Op = cast(this)->getOperator(); +if (CXXOperatorCallExpr::isAssignmentOp(Op)) { + const Decl *FD = cast(this)->getCalleeDecl(); aaron.ballman

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

aaron.ballman requested changes to this revision. This revision now requires changes to proceed. Comment at: include/clang/AST/ExprCXX.h:109 @@ -108,1 +108,3 @@ + // Check to see if a given overloaded operator is of assignment kind + static bool

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu marked 4 inline comments as done. Abpostelnicu added a comment. I've added also support for pure in case CXXOperatorCallExprClass::isAssignmentOperator is true. https://reviews.llvm.org/D22910 ___ cfe-commits mailing list

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu updated this revision to Diff 69205. https://reviews.llvm.org/D22910 Files: include/clang/AST/ExprCXX.h lib/AST/Expr.cpp Index: lib/AST/Expr.cpp === --- lib/AST/Expr.cpp +++ lib/AST/Expr.cpp @@ -2861,8 +2861,19 @@

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

rsmith added inline comments. Comment at: lib/AST/Expr.cpp:2865-2867 @@ +2864,5 @@ + case CXXOperatorCallExprClass: { +// If it is an operator call expr it can have side effects when the +// underlaying operator is of assignment kind. +// Othrwise fall through the

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu marked 2 inline comments as done. Comment at: lib/AST/Expr.cpp:2868 @@ +2867,3 @@ +OverloadedOperatorKind binOp = cast(this)->getOperator(); +if (binOp == OO_Equal || (binOp >= OO_PlusEqual && binOp <= OO_PipeEqual)) { + return true;

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu removed rL LLVM as the repository for this revision. Abpostelnicu updated this revision to Diff 68507. https://reviews.llvm.org/D22910 Files: include/clang/AST/ExprCXX.h lib/AST/Expr.cpp Index: lib/AST/Expr.cpp ===

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

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

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

Abpostelnicu added a comment. In https://reviews.llvm.org/D22910#499082, @aaron.ballman wrote: > Thank you for the patch! > > One thing this patch is missing is a test case that exercises this code path. > For instance, there are diagnostics triggered for expressions with side > effects when

Re: [PATCH] D22910: Add support for CXXOperatorCallExpr in Expr::HasSideEffects

aaron.ballman added a comment. Thank you for the patch! One thing this patch is missing is a test case that exercises this code path. For instance, there are diagnostics triggered for expressions with side effects when used as part of an unevaluated expression like sizeof, noexcept, etc. You