[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

This revision was automatically updated to reflect the committed changes. Closed by commit rL327939: [CodeGen] Ignore OpaqueValueExprs that are unique references to their (authored by ahatanak, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Alright, LGTM. https://reviews.llvm.org/D39562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak added a comment. I also add a call to setIsUnique(false) in OpaqueValueExpr's constructor, which fixed the failing tests I was seeing. https://reviews.llvm.org/D39562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak updated this revision to Diff 137418. ahatanak marked an inline comment as done. ahatanak added a comment. Don't set the IsUnique bit of an OpaqueValueExpr that is used as the result expression. https://reviews.llvm.org/D39562 Files: include/clang/AST/Expr.h

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak added inline comments. Comment at: lib/CodeGen/CGExpr.cpp:4815 +} + } + rjmccall wrote: > Oh! So it's an interesting point that the RHS might be used as the result > expression, which means its use might not really be unique anymore. It >

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak added a comment. Sorry, this patch is not ready yet. There are several regression tests that are failing because of the assert in setIsUnique. https://reviews.llvm.org/D39562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

rjmccall added inline comments. Comment at: include/clang/AST/Expr.h:875 + /// is set to true. + bool IsUnique = false; + rjmccall wrote: > Humor me and pack this in the bitfields in Stmt, please. :) Thanks! Comment at:

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak added inline comments. Comment at: lib/Sema/SemaPseudoObject.cpp:432 + if (capturedRHS->getType()->getAsCXXRecordDecl() && capturedRHS->isRValue()) +capturedRHS->setIsUnique(); + rjmccall wrote: > I think you can unconditionally set this here,

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak updated this revision to Diff 137250. ahatanak marked 3 inline comments as done. ahatanak added a comment. Address review comments. https://reviews.llvm.org/D39562 Files: include/clang/AST/Expr.h include/clang/AST/Stmt.h lib/CodeGen/CGExpr.cpp lib/CodeGen/CGExprAgg.cpp

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

rjmccall added a comment. Oh, and you need to serialize this bit. https://reviews.llvm.org/D39562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

rjmccall added inline comments. Comment at: include/clang/AST/Expr.h:875 + /// is set to true. + bool IsUnique = false; + Humor me and pack this in the bitfields in Stmt, please. :) Comment at: include/clang/AST/Expr.h:932 + void

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak updated this revision to Diff 136895. ahatanak added a comment. Add a flag to OpaqeuValueExpr that indicates whether it is a unique reference to its subexpression. If the flag is set, IRGen will avoid binding the OVE and copying the result to the destination and instead just visit the

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak added a comment. I'm planning to work on a new patch this week. https://reviews.llvm.org/D39562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

smeenai added a comment. Any updates here? We were hitting a similar error internally, which this patch fixed. https://reviews.llvm.org/D39562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

rjmccall added a comment. Hmm. The OVE-ing of the RHS of a property assignment is just there to make the original source structure clearer. Maybe the right solution here is to set a flag in the OVE that says that it's a unique semantic reference to its source expression, and then change

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak added a comment. This is tracked by rdar://problem/34363596. https://reviews.llvm.org/D39562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39562: [CodeGen][ObjC] Fix an assertion failure caused by copy elision

ahatanak created this revision. Herald added subscribers: kristof.beyls, aemerson. The assertion failure occurs when a setter is called using the dot syntax to set a property of a class type that doesn't have trivial copy/move constructors or assignment operators. It happens only when clang is