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 compiling for c++1z and copy elision allows avoiding copying a temporary to the argument passed to the setter method in Sema.
For example:
struct S2 {
id f1;
};
@interface C
@property S2 f;
@end
@implementation C
@end
void test(C *c) {
c.f = S2();
}When IRGen visits the ObjCMessageExpr for the call to the setter, it tries to emit a copy of an S2 object that has been constructed (this happens in AggExprEmitter::VisitOpaqueValueExpr) and the assertion in CodeGenFunction::EmitAggregateCopy fails because S2 doesn't have the trivial special functions needed to do the copy.
This is the AST of the ObjCMessageExpr:
`-ObjCMessageExpr 0x7fe03c06b730 <col:5> 'void' selector=setF: | |-OpaqueValueExpr 0x7fe03c06b698 <col:3> 'C *' | | `-ImplicitCastExpr 0x7fe03c06b5e0 <col:3> 'C *' <LValueToRValue> | | `-DeclRefExpr 0x7fe03c06b5b8 <col:3> 'C *__strong' lvalue ParmVar 0x7fe03c06b408 'c' 'C *__strong' | `-OpaqueValueExpr 0x7fe03c06b6e8 <col:9, col:12> 'struct S2' | `-CXXBindTemporaryExpr 0x7fe03c06b678 <col:9, col:12> 'struct S2' (CXXTemporary 0x7fe03c06b670) | `-CXXTemporaryObjectExpr 0x7fe03c06b638 <col:9, col:12> 'struct S2' 'void (void)'
To avoid the crash, I modified CodeGenFunction::EmitAnyExprToTemp to return the value for the OpaqueValueExpr (which must have already been evaluated when it's visited, I think) so that it doesn't have to call EmitAggregateCopy to make a copy.
I'm actually unsure whether we should fix Sema and change the AST representation or fix IRGen as I did in this patch. I'm open to suggestions if anyone has a better idea to fix the crash.
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 happens to work in some sense for non-trivial C++ class types (when they're passed indirectly, as under the Itanium ABI) because the temporary outlives the call; on the other hand, the call is allowed to mutate its argument, and it's not clear that it's okay to have those mutations be reflected in code that's using the result of the assignment. Similarly, managed types (like non-trivial C structs, or ARC pointers) might be consumed by the call; if we're going to pass them, perhaps we need to copy the value before doing so.
What do you think?
I think the technical implication is that what you're doing here shouldn't be necessary; the OVE arguably should not be unique if its value is used in multiple places, and that includes being used as a result.