diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h index e3e4ca224681..3de5cc921aa8 100644 --- a/clang/include/clang/AST/ExprCXX.h +++ b/clang/include/clang/AST/ExprCXX.h @@ -76,6 +76,16 @@ public: /// expression refers to. OverloadedOperatorKind getOperator() const { return Operator; } + static bool isAssignmentOp(OverloadedOperatorKind Opc) { + return Opc == OO_Equal || Opc == OO_StarEqual || + Opc == OO_SlashEqual || Opc == OO_PercentEqual || + Opc == OO_PlusEqual || Opc == OO_MinusEqual || + Opc == OO_LessLessEqual || Opc == OO_GreaterGreaterEqual || + Opc == OO_AmpEqual || Opc == OO_CaretEqual || + Opc == OO_PipeEqual; + } + bool isAssignmentOp() const { return isAssignmentOp(getOperator()); } + /// \brief Returns the location of the operator symbol in the expression. /// /// When \c getOperator()==OO_Call, this is the location of the right diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp index 4a24e42512af..90f172082845 100644 --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -3172,7 +3172,8 @@ void CodeGenFunction::EmitNonNullArgCheck(RValue RV, QualType ArgType, void CodeGenFunction::EmitCallArgs( CallArgList &Args, ArrayRef ArgTypes, llvm::iterator_range ArgRange, - const FunctionDecl *CalleeDecl, unsigned ParamsToSkip) { + const FunctionDecl *CalleeDecl, unsigned ParamsToSkip, + bool ForceRightToLeftEvaluation) { assert((int)ArgTypes.size() == (ArgRange.end() - ArgRange.begin())); auto MaybeEmitImplicitObjectSize = [&](unsigned I, const Expr *Arg) { @@ -3191,7 +3192,8 @@ void CodeGenFunction::EmitCallArgs( // We *have* to evaluate arguments from right to left in the MS C++ ABI, // because arguments are destroyed left to right in the callee. - if (CGM.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) { + if (CGM.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee() || + ForceRightToLeftEvaluation) { // Insert a stack save if we're going to need any inalloca args. bool HasInAllocaArgs = false; for (ArrayRef::iterator I = ArgTypes.begin(), E = ArgTypes.end(); diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index bca711fbc8ca..63aa34678504 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4121,8 +4121,17 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, llvm::Value *Callee, if (Chain) Args.add(RValue::get(Builder.CreateBitCast(Chain, CGM.VoidPtrTy)), CGM.getContext().VoidPtrTy); + + // C++17 requires that we evaluate arguments to a call using assignment syntax + // right-to-left. It also requires that we evaluate arguments to operators + // <<, >>, and ->* left-to-right, but that is not possible under the MS ABI, + // so there is no corresponding "force left-to-right" case. + bool ForceRightToLeft = false; + if (auto *OCE = dyn_cast(E)) + ForceRightToLeft = OCE->isAssignmentOp(); + EmitCallArgs(Args, dyn_cast(FnType), E->arguments(), - E->getDirectCallee(), /*ParamsToSkip*/ 0); + E->getDirectCallee(), /*ParamsToSkip*/ 0, ForceRightToLeft); const CGFunctionInfo &FnInfo = CGM.getTypes().arrangeFreeFunctionCall( Args, FnType, /*isChainCall=*/Chain); diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index 1dfe437c3392..34ab6adad2ac 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -28,7 +28,7 @@ static RequiredArgs commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, const CXXMethodDecl *MD, llvm::Value *This, llvm::Value *ImplicitParam, QualType ImplicitParamTy, const CallExpr *CE, - CallArgList &Args) { + CallArgList &Args, CallArgList *RtlArgs) { assert(CE == nullptr || isa(CE) || isa(CE)); assert(MD->isInstance() && @@ -61,7 +61,12 @@ commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, const CXXMethodDecl *MD, RequiredArgs required = RequiredArgs::forPrototypePlus(FPT, Args.size(), MD); // And the rest of the call args. - if (CE) { + if (RtlArgs) { + // Special case: if the caller emitted the arguments right-to-left already + // (prior to emitting the *this argument), we're done. This happens for + // assignment operators. + Args.addFrom(*RtlArgs); + } else if (CE) { // Special case: skip first argument of CXXOperatorCall (it is "this"). unsigned ArgsToSkip = isa(CE) ? 1 : 0; CGF.EmitCallArgs(Args, FPT, drop_begin(CE->arguments(), ArgsToSkip), @@ -77,11 +82,11 @@ commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, const CXXMethodDecl *MD, RValue CodeGenFunction::EmitCXXMemberOrOperatorCall( const CXXMethodDecl *MD, llvm::Value *Callee, ReturnValueSlot ReturnValue, llvm::Value *This, llvm::Value *ImplicitParam, QualType ImplicitParamTy, - const CallExpr *CE) { + const CallExpr *CE, CallArgList *RtlArgs) { const FunctionProtoType *FPT = MD->getType()->castAs(); CallArgList Args; RequiredArgs required = commonEmitCXXMemberOrOperatorCall( - *this, MD, This, ImplicitParam, ImplicitParamTy, CE, Args); + *this, MD, This, ImplicitParam, ImplicitParamTy, CE, Args, RtlArgs); return EmitCall(CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required), Callee, ReturnValue, Args, MD); } @@ -92,7 +97,7 @@ RValue CodeGenFunction::EmitCXXDestructorCall( StructorType Type) { CallArgList Args; commonEmitCXXMemberOrOperatorCall(*this, DD, This, ImplicitParam, - ImplicitParamTy, CE, Args); + ImplicitParamTy, CE, Args, nullptr); return EmitCall(CGM.getTypes().arrangeCXXStructorDeclaration(DD, Type), Callee, ReturnValueSlot(), Args, DD); } @@ -170,6 +175,19 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( } } + // C++17 demands that we evaluate the RHS of a (possibly-compound) assignment + // operator before the LHS. + CallArgList RtlArgStorage; + CallArgList *RtlArgs = nullptr; + if (auto *OCE = dyn_cast(CE)) { + if (OCE->isAssignmentOp()) { + RtlArgs = &RtlArgStorage; + EmitCallArgs(*RtlArgs, MD->getType()->castAs(), + drop_begin(CE->arguments(), 1), CE->getDirectCallee(), + /*ParamsToSkip*/0, /*ForceRightToLeftEvaluation*/true); + } + } + Address This = Address::invalid(); if (IsArrow) This = EmitPointerWithAlignment(Base); @@ -187,10 +205,12 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( if (MD->isCopyAssignmentOperator() || MD->isMoveAssignmentOperator()) { // We don't like to generate the trivial copy/move assignment operator // when it isn't necessary; just produce the proper effect here. - // Special case: skip first argument of CXXOperatorCall (it is "this"). - unsigned ArgsToSkip = isa(CE) ? 1 : 0; - Address RHS = EmitLValue(*(CE->arg_begin() + ArgsToSkip)).getAddress(); - EmitAggregateAssign(This, RHS, CE->getType()); + LValue RHS = isa(CE) + ? MakeNaturalAlignAddrLValue( + (*RtlArgs)[0].RV.getScalarVal(), + (*(CE->arg_begin() + 1))->getType()) + : EmitLValue(*CE->arg_begin()); + EmitAggregateAssign(This, RHS.getAddress(), CE->getType()); return RValue::get(This.getPointer()); } @@ -249,7 +269,8 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( Callee = CGM.GetAddrOfFunction(GlobalDecl(DDtor, Dtor_Complete), Ty); } EmitCXXMemberOrOperatorCall(MD, Callee, ReturnValue, This.getPointer(), - /*ImplicitParam=*/nullptr, QualType(), CE); + /*ImplicitParam=*/nullptr, QualType(), CE, + nullptr); } return RValue::get(nullptr); } @@ -282,7 +303,8 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( } return EmitCXXMemberOrOperatorCall(MD, Callee, ReturnValue, This.getPointer(), - /*ImplicitParam=*/nullptr, QualType(), CE); + /*ImplicitParam=*/nullptr, QualType(), CE, + RtlArgs); } RValue diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 7820fda6ba53..378a2562f8a3 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -2867,7 +2867,8 @@ public: EmitCXXMemberOrOperatorCall(const CXXMethodDecl *MD, llvm::Value *Callee, ReturnValueSlot ReturnValue, llvm::Value *This, llvm::Value *ImplicitParam, - QualType ImplicitParamTy, const CallExpr *E); + QualType ImplicitParamTy, const CallExpr *E, + CallArgList *RtlArgs); RValue EmitCXXDestructorCall(const CXXDestructorDecl *DD, llvm::Value *Callee, llvm::Value *This, llvm::Value *ImplicitParam, QualType ImplicitParamTy, const CallExpr *E, @@ -3322,7 +3323,8 @@ public: void EmitCallArgs(CallArgList &Args, const T *CallArgTypeInfo, llvm::iterator_range ArgRange, const FunctionDecl *CalleeDecl = nullptr, - unsigned ParamsToSkip = 0) { + unsigned ParamsToSkip = 0, + bool ForceRightToLeftEvaluation = false) { SmallVector ArgTypes; CallExpr::const_arg_iterator Arg = ArgRange.begin(); @@ -3362,13 +3364,15 @@ public: for (auto *A : llvm::make_range(Arg, ArgRange.end())) ArgTypes.push_back(getVarArgType(A)); - EmitCallArgs(Args, ArgTypes, ArgRange, CalleeDecl, ParamsToSkip); + EmitCallArgs(Args, ArgTypes, ArgRange, CalleeDecl, ParamsToSkip, + ForceRightToLeftEvaluation); } void EmitCallArgs(CallArgList &Args, ArrayRef ArgTypes, llvm::iterator_range ArgRange, const FunctionDecl *CalleeDecl = nullptr, - unsigned ParamsToSkip = 0); + unsigned ParamsToSkip = 0, + bool ForceRightToLeftEvaluation = false); /// EmitPointerWithAlignment - Given an expression with a pointer /// type, emit the value and compute our best estimate of the diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 08c86176abcf..fd8ef25a05e1 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -1456,7 +1456,8 @@ void ItaniumCXXABI::EmitDestructorCall(CodeGenFunction &CGF, Callee = CGM.getAddrOfCXXStructor(DD, getFromDtorType(Type)); CGF.EmitCXXMemberOrOperatorCall(DD, Callee, ReturnValueSlot(), - This.getPointer(), VTT, VTTTy, nullptr); + This.getPointer(), VTT, VTTTy, + nullptr, nullptr); } void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT, @@ -1636,7 +1637,7 @@ llvm::Value *ItaniumCXXABI::EmitVirtualDestructorCall( CGF.EmitCXXMemberOrOperatorCall(Dtor, Callee, ReturnValueSlot(), This.getPointer(), /*ImplicitParam=*/nullptr, - QualType(), CE); + QualType(), CE, nullptr); return nullptr; } diff --git a/clang/test/CodeGenCXX/cxx1z-eval-order.cpp b/clang/test/CodeGenCXX/cxx1z-eval-order.cpp index f6cfb2b8fd81..e482320ae8fc 100644 --- a/clang/test/CodeGenCXX/cxx1z-eval-order.cpp +++ b/clang/test/CodeGenCXX/cxx1z-eval-order.cpp @@ -18,7 +18,8 @@ struct B { struct C { operator int *(); A *operator->(); - void operator->*(B); + void operator->*(A); + friend void operator->*(C, B); friend void operator<<(C, B); friend void operator>>(C, B); @@ -137,7 +138,17 @@ int dotstar_lhs_before_rhs() { int b = make_a_ptr()->*make_mem_ptr_a(); // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( + make_c()->*make_a(); + + // FIXME: The corresponding case for Windows ABIs is unimplementable if the + // operand has a non-trivially-destructible type, because the order of + // construction of function arguments is defined by the ABI there (it's the + // reverse of the order in which the parameters are destroyed in the callee). + // But we could follow the C++17 rule in the case where the operand type is + // trivially-destructible. + // CHECK-ITANIUM: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK-ITANIUM: call {{.*}}@{{.*}}make_b{{.*}}( make_c()->*make_b(); // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( @@ -154,61 +165,60 @@ int dotstar_lhs_before_rhs() { // CHECK: } } -#if 0 -// CHECKDISABLED-LABEL: define {{.*}}@{{.*}}assign_lhs_before_rhs{{.*}}( + +// CHECK-LABEL: define {{.*}}@{{.*}}assign_rhs_before_lhs{{.*}}( void assign_rhs_before_lhs() { extern int &lhs_ref(), rhs(); - // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}lhs_ref{{.*}}( + // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECK: call {{.*}}@{{.*}}lhs_ref{{.*}}( lhs_ref() = rhs(); - // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}lhs_ref{{.*}}( + // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECK: call {{.*}}@{{.*}}lhs_ref{{.*}}( lhs_ref() += rhs(); - // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}lhs_ref{{.*}}( + // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECK: call {{.*}}@{{.*}}lhs_ref{{.*}}( lhs_ref() %= rhs(); - // CHECKDISABLED: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( make_c() = make_b(); - // CHECKDISABLED: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( make_c() += make_b(); -// CHECKDISABLED: } +// CHECK: } } -#endif -#if 0 -// CHECKDISABLED-LABEL: define {{.*}}@{{.*}}shift_lhs_before_rhs{{.*}}( + +// CHECK-LABEL: define {{.*}}@{{.*}}shift_lhs_before_rhs{{.*}}( void shift_lhs_before_rhs() { extern int lhs(), rhs(); - // CHECKDISABLED: call {{.*}}@{{.*}}lhs{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECK: call {{.*}}@{{.*}}lhs{{.*}}( + // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( (void)(lhs() << rhs()); - // CHECKDISABLED: call {{.*}}@{{.*}}lhs{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECK: call {{.*}}@{{.*}}lhs{{.*}}( + // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( (void)(lhs() >> rhs()); - // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}make_a{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( make_c() << make_a(); - // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}make_a{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( make_c() >> make_a(); - // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}make_b{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( make_c() << make_b(); - // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECKDISABLED: call {{.*}}@{{.*}}make_b{{.*}}( + // FIXME: This is unimplementable for Windows ABIs, see above. + // CHECK-ITANIUM: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECK-ITANIUM: call {{.*}}@{{.*}}make_b{{.*}}( make_c() >> make_b(); -// CHECKDISABLED: } +// CHECK: } } -#endif diff --git a/clang/test/CodeGenObjCXX/property-object-reference-2.mm b/clang/test/CodeGenObjCXX/property-object-reference-2.mm index 925cc52f9092..230fe485895f 100644 --- a/clang/test/CodeGenObjCXX/property-object-reference-2.mm +++ b/clang/test/CodeGenObjCXX/property-object-reference-2.mm @@ -44,8 +44,8 @@ struct TCPPObject // CHECK: ret void // CHECK-LABEL: define internal void @__assign_helper_atomic_property_(%struct.TCPPObject*, %struct.TCPPObject*) # -// CHECK: [[TWO:%.*]] = load %struct.TCPPObject*, %struct.TCPPObject** [[ADDR:%.*]], align 8 // CHECK: [[THREE:%.*]] = load %struct.TCPPObject*, %struct.TCPPObject** [[ADDR1:%.*]], align 8 +// CHECK: [[TWO:%.*]] = load %struct.TCPPObject*, %struct.TCPPObject** [[ADDR:%.*]], align 8 // CHECK: [[CALL:%.*]] = call dereferenceable({{[0-9]+}}) %struct.TCPPObject* @_ZN10TCPPObjectaSERKS_(%struct.TCPPObject* [[TWO]], %struct.TCPPObject* dereferenceable({{[0-9]+}}) [[THREE]]) // CHECK: ret void diff --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html index b22ec4598af8..1857f1264cba 100644 --- a/clang/www/cxx_status.html +++ b/clang/www/cxx_status.html @@ -701,7 +701,7 @@ as the draft C++1z standard evolves.

Stricter expression evaluation order P0145R3 - No + SVN (10) P0400R0 @@ -741,6 +741,14 @@ In Clang 3.7, a warning is emitted for all cases that would change meaning.
(9): This is the resolution to a Defect Report, so is applied to all language versions supporting inheriting constructors. +
+(10): Under the MS ABI, this feature is not fully implementable, +because the calling convention requires that function parameters are destroyed +from left to right in the callee. In order to guarantee that destruction order +is reverse construction order, the operands of overloaded +operator<<, operator>>, and operator->* +functions are evaluated right-to-left under the MS ABI when called using operator +syntax, not left-to-right as P0145R3 requires.