Diff Detail
Event Timeline
Thanks for the patch!
Side-comment: I wonder if ever need to copy-elide OpaqueValues. There could be similar issues that happen to OpaqueValue expression that we haven't uncovered yet.
lib/Sema/SemaExprCXX.cpp | ||
---|---|---|
5637 | Have you tried checking if LHS and Cond are the same instead of using the OpaqueValue to check for the BinaryConditionalOperator? The comment at the top of the function states that for gnu ?: operators LHS == Cond should be true. | |
test/CodeGenCXX/gnu-conditional-extension.cpp | ||
1 | Is it possible to create a C++1z Sema test as well? E.g. make copy & move constructor private for a struct so it should fail to compile when we try to copy the LHS? |
Actually, I submitted radar://35088001 which looks like this issues as well. In that case it was a problem with assigning a C++ object that's a property in a ObjC class using dot syntax E.g.
@interface A @property CXXClass myCXXClass; @end ... A *a a.myCXXClass = makeCXXClass(); ...
Doing a simple check for OpaqueValues in TryConstructorInitialization appears to fix both problems and would eliminate the need for the special handling of each case (assuming we never elide OpaqueValues).
I'm working on an alternative approach to fix this bug:
Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp (revision 316821) +++ lib/Sema/SemaExpr.cpp (working copy) @@ -7242,6 +7242,16 @@ commonExpr = commonRes.get(); } + // If the common expression is a class or array prvalue, materialize it + // so that we can safely refer to it multiple times. + if (commonExpr->isRValue() && (commonExpr->getType()->isClassType() || + commonExpr->getType()->isArrayType())) { + ExprResult MatExpr = TemporaryMaterializationConversion(commonExpr); + if (MatExpr.isInvalid()) + return ExprError(); + commonExpr = MatExpr.get(); + } + opaqueValue = new (Context) OpaqueValueExpr(commonExpr->getExprLoc(), commonExpr->getType(), commonExpr->getValueKind(),
Have you tried checking if LHS and Cond are the same instead of using the OpaqueValue to check for the BinaryConditionalOperator? The comment at the top of the function states that for gnu ?: operators LHS == Cond should be true.