Author: rjmccall Date: Fri Aug 21 19:35:27 2015 New Revision: 245771 URL: http://llvm.org/viewvc/llvm-project?rev=245771&view=rev Log: When building a pseudo-object assignment, and the RHS is a contextually-typed expression that semantic analysis will probably need to invasively rewrite, don't include the RHS OVE as a separate semantic expression, and check the operation with the original RHS expression.
There are two contextually-typed expressions that can survive to here: overloaded function references, which are at least safe to double-emit, and C++11 initializer list expressions, which are not at all safe to double-emit and which often don't update the original syntactic InitListExpr with implicit conversions to member types, etc. This means that the original RHS may appear, undecorated by an OVE, in the semantic expressions. Fortunately, it will only ever be used in a single place there, and I don't believe there are clients that rely on being able to pick out the original RHS from the semantic expressions. But this could be problematic if there are clients that do visit the entire tree and rely on not seeing the same expression multiple times, once in the syntactic and once in the semantic expressions. This is a very fiddly part of the compiler. rdar://21801088 Modified: cfe/trunk/lib/Sema/SemaPseudoObject.cpp cfe/trunk/test/CodeGenObjCXX/property-objects.mm Modified: cfe/trunk/lib/Sema/SemaPseudoObject.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaPseudoObject.cpp?rev=245771&r1=245770&r2=245771&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaPseudoObject.cpp (original) +++ cfe/trunk/lib/Sema/SemaPseudoObject.cpp Fri Aug 21 19:35:27 2015 @@ -406,19 +406,27 @@ PseudoOpBuilder::buildAssignmentOperatio BinaryOperatorKind opcode, Expr *LHS, Expr *RHS) { assert(BinaryOperator::isAssignmentOp(opcode)); - - // Recover from user error - if (isa<UnresolvedLookupExpr>(RHS)) - return ExprError(); Expr *syntacticLHS = rebuildAndCaptureObject(LHS); OpaqueValueExpr *capturedRHS = capture(RHS); + // In some very specific cases, semantic analysis of the RHS as an + // expression may require it to be rewritten. In these cases, we + // cannot safely keep the OVE around. Fortunately, we don't really + // need to: we don't use this particular OVE in multiple places, and + // no clients rely that closely on matching up expressions in the + // semantic expression with expressions from the syntactic form. + Expr *semanticRHS = capturedRHS; + if (RHS->hasPlaceholderType() || isa<InitListExpr>(RHS)) { + semanticRHS = RHS; + Semantics.pop_back(); + } + Expr *syntactic; ExprResult result; if (opcode == BO_Assign) { - result = capturedRHS; + result = semanticRHS; syntactic = new (S.Context) BinaryOperator(syntacticLHS, capturedRHS, opcode, capturedRHS->getType(), capturedRHS->getValueKind(), @@ -430,8 +438,7 @@ PseudoOpBuilder::buildAssignmentOperatio // Build an ordinary, non-compound operation. BinaryOperatorKind nonCompound = BinaryOperator::getOpForCompoundAssignment(opcode); - result = S.BuildBinOp(Sc, opcLoc, nonCompound, - opLHS.get(), capturedRHS); + result = S.BuildBinOp(Sc, opcLoc, nonCompound, opLHS.get(), semanticRHS); if (result.isInvalid()) return ExprError(); syntactic = @@ -745,16 +752,6 @@ ExprResult ObjCPropertyOpBuilder::buildS op = opResult.get(); assert(op && "successful assignment left argument invalid?"); } - else if (OpaqueValueExpr *OVE = dyn_cast<OpaqueValueExpr>(op)) { - Expr *Initializer = OVE->getSourceExpr(); - // passing C++11 style initialized temporaries to objc++ properties - // requires special treatment by removing OpaqueValueExpr so type - // conversion takes place and adding the OpaqueValueExpr later on. - if (isa<InitListExpr>(Initializer) && - Initializer->getType()->isVoidType()) { - op = Initializer; - } - } } // Arguments. Modified: cfe/trunk/test/CodeGenObjCXX/property-objects.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/property-objects.mm?rev=245771&r1=245770&r2=245771&view=diff ============================================================================== --- cfe/trunk/test/CodeGenObjCXX/property-objects.mm (original) +++ cfe/trunk/test/CodeGenObjCXX/property-objects.mm Fri Aug 21 19:35:27 2015 @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -emit-llvm -g -o - | FileCheck %s +// RUN: %clang_cc1 %s -triple=x86_64-apple-darwin10 -std=c++11 -emit-llvm -g -o - | FileCheck %s class S { public: @@ -91,3 +91,133 @@ struct X { void f(A* a) { a.x = X(); } + +// rdar://21801088 +// Ensure that pseudo-objecet expressions that require the RHS to be +// rewritten don't result in crashes or redundant emission of code. +struct B0 { long long x; }; +struct B1 { long long x; }; B1 operator+(B1, B1); +struct B2 { B1 x; }; +struct B3 { B3(); B1 x; operator B1(); }; +@interface B +@property B0 b0; +@property B1 b1; +@property B2 b2; +@property B3 b3; +@end + +int b_makeInt(); + +// Note that there's a promotion from int to long long, so +// the syntactic form of the RHS will be bogus. +void testB0(B *b) { + b.b0 = { b_makeInt() }; +} +void testB1(B *b) { + b.b1 += { b_makeInt() }; +} +// CHECK: define void @_Z6testB0P1B([[B:%.*]]* +// CHECK: [[BVAR:%.*]] = alloca [[B]]*, align 8 +// CHECK: [[TEMP:%.*]] = alloca [[B0:%.*]], align 8 +// CHECK: load [[B]]*, [[B]]** [[BVAR]] +// CHECK-NEXT: [[X:%.*]] = getelementptr inbounds [[B0]], [[B0]]* [[TEMP]], i32 0, i32 0 +// CHECK-NEXT: [[T0:%.*]] = call i32 @_Z9b_makeIntv() +// CHECK-NEXT: [[T1:%.*]] = sext i32 [[T0]] to i64 +// CHECK-NEXT: store i64 [[T1]], i64* [[X]], align 8 +// CHECK-NOT: call +// CHECK: call void @llvm.memcpy +// CHECK-NOT: call +// CHECK: call void bitcast {{.*}} @objc_msgSend +// CHECK-NOT: call +// CHECK: ret void + +// CHECK: define void @_Z6testB1P1B([[B]]* +// CHECK: [[BVAR:%.*]] = alloca [[B]]*, align 8 +// CHECK: load [[B]]*, [[B]]** [[BVAR]] +// CHECK-NOT: call +// CHECK: [[T0:%.*]] = call i64 bitcast {{.*}} @objc_msgSend +// CHECK-NOT: call +// CHECK: store i64 [[T0]], +// CHECK-NOT: call +// CHECK: [[T0:%.*]] = call i32 @_Z9b_makeIntv() +// CHECK-NEXT: [[T1:%.*]] = sext i32 [[T0]] to i64 +// CHECK-NEXT: store i64 [[T1]], i64* {{.*}}, align 8 +// CHECK-NOT: call +// CHECK: [[T0:%.*]] = call i64 @_Zpl2B1S_ +// CHECK-NOT: call +// CHECK: store i64 [[T0]], +// CHECK-NOT: call +// CHECK: call void @llvm.memcpy +// CHECK-NOT: call +// CHECK: call void bitcast {{.*}} @objc_msgSend +// CHECK-NOT: call +// CHECK: ret void + +// Another example of a conversion that needs to be applied +// in the semantic form. +void testB2(B *b) { + b.b2 = { B3() }; +} + +// CHECK: define void @_Z6testB2P1B([[B]]* +// CHECK: [[BVAR:%.*]] = alloca [[B]]*, align 8 +// CHECK: load [[B]]*, [[B]]** [[BVAR]] +// CHECK-NOT: call +// CHECK: call void @_ZN2B3C1Ev( +// CHECK-NEXT: [[T0:%.*]] = call i64 @_ZN2B3cv2B1Ev( +// CHECK-NOT: call +// CHECK: store i64 [[T0]], +// CHECK-NOT: call +// CHECK: call void @llvm.memcpy +// CHECK-NOT: call +// CHECK: call void bitcast {{.*}} @objc_msgSend +// CHECK-NOT: call +// CHECK: ret void + +// A similar test to B, but using overloaded function references. +struct C1 { + int x; + friend C1 operator+(C1, void(&)()); +}; +@interface C +@property void (*c0)(); +@property C1 c1; +@end + +void c_helper(); +void c_helper(int); + +void testC0(C *c) { + c.c0 = c_helper; + c.c0 = &c_helper; +} +// CHECK: define void @_Z6testC0P1C([[C:%.*]]* +// CHECK: [[CVAR:%.*]] = alloca [[C]]*, align 8 +// CHECK: load [[C]]*, [[C]]** [[CVAR]] +// CHECK-NOT: call +// CHECK: call void bitcast {{.*}} @objc_msgSend {{.*}} @_Z8c_helperv +// CHECK-NOT: call +// CHECK: call void bitcast {{.*}} @objc_msgSend {{.*}} @_Z8c_helperv +// CHECK-NOT: call +// CHECK: ret void + +void testC1(C *c) { + c.c1 += c_helper; +} +// CHECK: define void @_Z6testC1P1C([[C]]* +// CHECK: [[CVAR:%.*]] = alloca [[C]]*, align 8 +// CHECK: load [[C]]*, [[C]]** [[CVAR]] +// CHECK-NOT: call +// CHECK: [[T0:%.*]] = call i32 bitcast {{.*}} @objc_msgSend +// CHECK-NOT: call +// CHECK: store i32 [[T0]], +// CHECK-NOT: call +// CHECK: [[T0:%.*]] = call i32 @_Zpl2C1RFvvE({{.*}} @_Z8c_helperv +// CHECK-NOT: call +// CHECK: store i32 [[T0]], +// CHECK-NOT: call +// CHECK: call void @llvm.memcpy +// CHECK-NOT: call +// CHECK: call void bitcast {{.*}} @objc_msgSend +// CHECK-NOT: call +// CHECK: ret void _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits