From 1ec534d937d149ebfd742f0158767cfada2bd68e Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Wed, 28 Sep 2016 02:20:06 +0000 Subject: [PATCH] Revert r282556. This change made several bots unhappy. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@282564 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/ExprCXX.h | 10 --- lib/CodeGen/CGCall.cpp | 6 +- lib/CodeGen/CGExpr.cpp | 11 +-- lib/CodeGen/CGExprCXX.cpp | 44 +++-------- lib/CodeGen/CodeGenFunction.h | 12 +-- lib/CodeGen/ItaniumCXXABI.cpp | 5 +- test/CodeGenCXX/cxx1z-eval-order.cpp | 74 ++++++++----------- .../property-object-reference-2.mm | 2 +- www/cxx_status.html | 10 +-- 9 files changed, 54 insertions(+), 120 deletions(-) diff --git a/include/clang/AST/ExprCXX.h b/include/clang/AST/ExprCXX.h index 3de5cc921a..e3e4ca2246 100644 --- a/include/clang/AST/ExprCXX.h +++ b/include/clang/AST/ExprCXX.h @@ -76,16 +76,6 @@ 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/lib/CodeGen/CGCall.cpp b/lib/CodeGen/CGCall.cpp index 90f1720828..4a24e42512 100644 --- a/lib/CodeGen/CGCall.cpp +++ b/lib/CodeGen/CGCall.cpp @@ -3172,8 +3172,7 @@ void CodeGenFunction::EmitNonNullArgCheck(RValue RV, QualType ArgType, void CodeGenFunction::EmitCallArgs( CallArgList &Args, ArrayRef ArgTypes, llvm::iterator_range ArgRange, - const FunctionDecl *CalleeDecl, unsigned ParamsToSkip, - bool ForceRightToLeftEvaluation) { + const FunctionDecl *CalleeDecl, unsigned ParamsToSkip) { assert((int)ArgTypes.size() == (ArgRange.end() - ArgRange.begin())); auto MaybeEmitImplicitObjectSize = [&](unsigned I, const Expr *Arg) { @@ -3192,8 +3191,7 @@ 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() || - ForceRightToLeftEvaluation) { + if (CGM.getTarget().getCXXABI().areArgsDestroyedLeftToRightInCallee()) { // 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/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index 63aa346785..bca711fbc8 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -4121,17 +4121,8 @@ 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, ForceRightToLeft); + E->getDirectCallee(), /*ParamsToSkip*/ 0); const CGFunctionInfo &FnInfo = CGM.getTypes().arrangeFreeFunctionCall( Args, FnType, /*isChainCall=*/Chain); diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp index 34ab6adad2..1dfe437c33 100644 --- a/lib/CodeGen/CGExprCXX.cpp +++ b/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 *RtlArgs) { + CallArgList &Args) { assert(CE == nullptr || isa(CE) || isa(CE)); assert(MD->isInstance() && @@ -61,12 +61,7 @@ commonEmitCXXMemberOrOperatorCall(CodeGenFunction &CGF, const CXXMethodDecl *MD, RequiredArgs required = RequiredArgs::forPrototypePlus(FPT, Args.size(), MD); // And the rest of the call args. - 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) { + 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), @@ -82,11 +77,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, CallArgList *RtlArgs) { + const CallExpr *CE) { const FunctionProtoType *FPT = MD->getType()->castAs(); CallArgList Args; RequiredArgs required = commonEmitCXXMemberOrOperatorCall( - *this, MD, This, ImplicitParam, ImplicitParamTy, CE, Args, RtlArgs); + *this, MD, This, ImplicitParam, ImplicitParamTy, CE, Args); return EmitCall(CGM.getTypes().arrangeCXXMethodCall(Args, FPT, required), Callee, ReturnValue, Args, MD); } @@ -97,7 +92,7 @@ RValue CodeGenFunction::EmitCXXDestructorCall( StructorType Type) { CallArgList Args; commonEmitCXXMemberOrOperatorCall(*this, DD, This, ImplicitParam, - ImplicitParamTy, CE, Args, nullptr); + ImplicitParamTy, CE, Args); return EmitCall(CGM.getTypes().arrangeCXXStructorDeclaration(DD, Type), Callee, ReturnValueSlot(), Args, DD); } @@ -175,19 +170,6 @@ 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); @@ -205,12 +187,10 @@ 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. - LValue RHS = isa(CE) - ? MakeNaturalAlignAddrLValue( - (*RtlArgs)[0].RV.getScalarVal(), - (*(CE->arg_begin() + 1))->getType()) - : EmitLValue(*CE->arg_begin()); - EmitAggregateAssign(This, RHS.getAddress(), CE->getType()); + // 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()); return RValue::get(This.getPointer()); } @@ -269,8 +249,7 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( Callee = CGM.GetAddrOfFunction(GlobalDecl(DDtor, Dtor_Complete), Ty); } EmitCXXMemberOrOperatorCall(MD, Callee, ReturnValue, This.getPointer(), - /*ImplicitParam=*/nullptr, QualType(), CE, - nullptr); + /*ImplicitParam=*/nullptr, QualType(), CE); } return RValue::get(nullptr); } @@ -303,8 +282,7 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr( } return EmitCXXMemberOrOperatorCall(MD, Callee, ReturnValue, This.getPointer(), - /*ImplicitParam=*/nullptr, QualType(), CE, - RtlArgs); + /*ImplicitParam=*/nullptr, QualType(), CE); } RValue diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index 378a2562f8..7820fda6ba 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -2867,8 +2867,7 @@ public: EmitCXXMemberOrOperatorCall(const CXXMethodDecl *MD, llvm::Value *Callee, ReturnValueSlot ReturnValue, llvm::Value *This, llvm::Value *ImplicitParam, - QualType ImplicitParamTy, const CallExpr *E, - CallArgList *RtlArgs); + QualType ImplicitParamTy, const CallExpr *E); RValue EmitCXXDestructorCall(const CXXDestructorDecl *DD, llvm::Value *Callee, llvm::Value *This, llvm::Value *ImplicitParam, QualType ImplicitParamTy, const CallExpr *E, @@ -3323,8 +3322,7 @@ public: void EmitCallArgs(CallArgList &Args, const T *CallArgTypeInfo, llvm::iterator_range ArgRange, const FunctionDecl *CalleeDecl = nullptr, - unsigned ParamsToSkip = 0, - bool ForceRightToLeftEvaluation = false) { + unsigned ParamsToSkip = 0) { SmallVector ArgTypes; CallExpr::const_arg_iterator Arg = ArgRange.begin(); @@ -3364,15 +3362,13 @@ public: for (auto *A : llvm::make_range(Arg, ArgRange.end())) ArgTypes.push_back(getVarArgType(A)); - EmitCallArgs(Args, ArgTypes, ArgRange, CalleeDecl, ParamsToSkip, - ForceRightToLeftEvaluation); + EmitCallArgs(Args, ArgTypes, ArgRange, CalleeDecl, ParamsToSkip); } void EmitCallArgs(CallArgList &Args, ArrayRef ArgTypes, llvm::iterator_range ArgRange, const FunctionDecl *CalleeDecl = nullptr, - unsigned ParamsToSkip = 0, - bool ForceRightToLeftEvaluation = false); + unsigned ParamsToSkip = 0); /// EmitPointerWithAlignment - Given an expression with a pointer /// type, emit the value and compute our best estimate of the diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp index fd8ef25a05..08c86176ab 100644 --- a/lib/CodeGen/ItaniumCXXABI.cpp +++ b/lib/CodeGen/ItaniumCXXABI.cpp @@ -1456,8 +1456,7 @@ void ItaniumCXXABI::EmitDestructorCall(CodeGenFunction &CGF, Callee = CGM.getAddrOfCXXStructor(DD, getFromDtorType(Type)); CGF.EmitCXXMemberOrOperatorCall(DD, Callee, ReturnValueSlot(), - This.getPointer(), VTT, VTTTy, - nullptr, nullptr); + This.getPointer(), VTT, VTTTy, nullptr); } void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT, @@ -1637,7 +1636,7 @@ llvm::Value *ItaniumCXXABI::EmitVirtualDestructorCall( CGF.EmitCXXMemberOrOperatorCall(Dtor, Callee, ReturnValueSlot(), This.getPointer(), /*ImplicitParam=*/nullptr, - QualType(), CE, nullptr); + QualType(), CE); return nullptr; } diff --git a/test/CodeGenCXX/cxx1z-eval-order.cpp b/test/CodeGenCXX/cxx1z-eval-order.cpp index e482320ae8..f6cfb2b8fd 100644 --- a/test/CodeGenCXX/cxx1z-eval-order.cpp +++ b/test/CodeGenCXX/cxx1z-eval-order.cpp @@ -18,8 +18,7 @@ struct B { struct C { operator int *(); A *operator->(); - void operator->*(A); - friend void operator->*(C, B); + void operator->*(B); friend void operator<<(C, B); friend void operator>>(C, B); @@ -138,17 +137,7 @@ int dotstar_lhs_before_rhs() { int b = make_a_ptr()->*make_mem_ptr_a(); // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( - // 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{{.*}}( + // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( make_c()->*make_b(); // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( @@ -165,60 +154,61 @@ int dotstar_lhs_before_rhs() { // CHECK: } } - -// CHECK-LABEL: define {{.*}}@{{.*}}assign_rhs_before_lhs{{.*}}( +#if 0 +// CHECKDISABLED-LABEL: define {{.*}}@{{.*}}assign_lhs_before_rhs{{.*}}( void assign_rhs_before_lhs() { extern int &lhs_ref(), rhs(); - // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( - // CHECK: call {{.*}}@{{.*}}lhs_ref{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}lhs_ref{{.*}}( lhs_ref() = rhs(); - // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( - // CHECK: call {{.*}}@{{.*}}lhs_ref{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}lhs_ref{{.*}}( lhs_ref() += rhs(); - // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( - // CHECK: call {{.*}}@{{.*}}lhs_ref{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}lhs_ref{{.*}}( lhs_ref() %= rhs(); - // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_b{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( make_c() = make_b(); - // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( - // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_b{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( make_c() += make_b(); -// CHECK: } +// CHECKDISABLED: } } - -// CHECK-LABEL: define {{.*}}@{{.*}}shift_lhs_before_rhs{{.*}}( +#endif +#if 0 +// CHECKDISABLED-LABEL: define {{.*}}@{{.*}}shift_lhs_before_rhs{{.*}}( void shift_lhs_before_rhs() { extern int lhs(), rhs(); - // CHECK: call {{.*}}@{{.*}}lhs{{.*}}( - // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}lhs{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( (void)(lhs() << rhs()); - // CHECK: call {{.*}}@{{.*}}lhs{{.*}}( - // CHECK: call {{.*}}@{{.*}}rhs{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}lhs{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}rhs{{.*}}( (void)(lhs() >> rhs()); - // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_a{{.*}}( make_c() << make_a(); - // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK: call {{.*}}@{{.*}}make_a{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_a{{.*}}( make_c() >> make_a(); - // CHECK: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK: call {{.*}}@{{.*}}make_b{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_b{{.*}}( make_c() << make_b(); - // FIXME: This is unimplementable for Windows ABIs, see above. - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_c{{.*}}( - // CHECK-ITANIUM: call {{.*}}@{{.*}}make_b{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_c{{.*}}( + // CHECKDISABLED: call {{.*}}@{{.*}}make_b{{.*}}( make_c() >> make_b(); -// CHECK: } +// CHECKDISABLED: } } +#endif diff --git a/test/CodeGenObjCXX/property-object-reference-2.mm b/test/CodeGenObjCXX/property-object-reference-2.mm index 230fe48589..925cc52f90 100644 --- a/test/CodeGenObjCXX/property-object-reference-2.mm +++ b/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: [[THREE:%.*]] = load %struct.TCPPObject*, %struct.TCPPObject** [[ADDR1:%.*]], align 8 // CHECK: [[TWO:%.*]] = load %struct.TCPPObject*, %struct.TCPPObject** [[ADDR:%.*]], align 8 +// CHECK: [[THREE:%.*]] = load %struct.TCPPObject*, %struct.TCPPObject** [[ADDR1:%.*]], 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/www/cxx_status.html b/www/cxx_status.html index 1857f1264c..b22ec4598a 100644 --- a/www/cxx_status.html +++ b/www/cxx_status.html @@ -701,7 +701,7 @@ as the draft C++1z standard evolves.

Stricter expression evaluation order P0145R3 - SVN (10) + No P0400R0 @@ -741,14 +741,6 @@ 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.

-- 2.50.1