maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

P0145R3 (C++17 evaluation order tweaks): evaluate the right-hand side of 

assignment and compound-assignment operators before the left-hand side. (Even
if it's an overloaded operator.)

This completes the implementation of P0145R3 + P0400R0 for all targets except
Windows, where the evaluation order guarantees for <<, >>, and ->* are
unimplementable as the ABI requires the function arguments are evaluated from
right to left (because parameter destructors are run from left to right in the
callee).

llvm-svn: 282556
This commit is contained in:
Richard Smith 2016-09-27 23:44:22 +00:00
parent 3d1235a97d
commit 97a616d624
9 changed files with 120 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
clang/include/clang/AST/ExprCXX.h
View File

@ -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

6
clang/lib/CodeGen/CGCall.cpp
View File

@ -3172,7 +3172,8 @@ void CodeGenFunction::EmitNonNullArgCheck(RValue RV, QualType ArgType,
void CodeGenFunction::EmitCallArgs(
CallArgList &Args, ArrayRef<QualType> ArgTypes,
llvm::iterator_range<CallExpr::const_arg_iterator> 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<QualType>::iterator I = ArgTypes.begin(), E = ArgTypes.end();

11
clang/lib/CodeGen/CGExpr.cpp
View File

@ -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<CXXOperatorCallExpr>(E))
ForceRightToLeft = OCE->isAssignmentOp();
EmitCallArgs(Args, dyn_cast<FunctionProtoType>(FnType), E->arguments(),
E->getDirectCallee(), /*ParamsToSkip*/ 0);
E->getDirectCallee(), /*ParamsToSkip*/ 0, ForceRightToLeft);
const CGFunctionInfo &FnInfo = CGM.getTypes().arrangeFreeFunctionCall(
Args, FnType, /*isChainCall=*/Chain);

44
clang/lib/CodeGen/CGExprCXX.cpp
View File

@ -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<CXXMemberCallExpr>(CE) ||
isa<CXXOperatorCallExpr>(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<CXXOperatorCallExpr>(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<FunctionProtoType>();
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<CXXOperatorCallExpr>(CE)) {
if (OCE->isAssignmentOp()) {
RtlArgs = &RtlArgStorage;
EmitCallArgs(*RtlArgs, MD->getType()->castAs<FunctionProtoType>(),
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<CXXOperatorCallExpr>(CE) ? 1 : 0;
Address RHS = EmitLValue(*(CE->arg_begin() + ArgsToSkip)).getAddress();
EmitAggregateAssign(This, RHS, CE->getType());
LValue RHS = isa<CXXOperatorCallExpr>(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

12
clang/lib/CodeGen/CodeGenFunction.h
View File

@ -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<CallExpr::const_arg_iterator> ArgRange,
const FunctionDecl *CalleeDecl = nullptr,
unsigned ParamsToSkip = 0) {
unsigned ParamsToSkip = 0,
bool ForceRightToLeftEvaluation = false) {
SmallVector<QualType, 16> 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<QualType> ArgTypes,
llvm::iterator_range<CallExpr::const_arg_iterator> 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

5
clang/lib/CodeGen/ItaniumCXXABI.cpp
View File

@ -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;
}

74
clang/test/CodeGenCXX/cxx1z-eval-order.cpp
View File

@ -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

2
clang/test/CodeGenObjCXX/property-object-reference-2.mm
View File

@ -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

10
clang/www/cxx_status.html
View File

@ -701,7 +701,7 @@ as the draft C++1z standard evolves.</p>
<tr>
<td rowspan=2>Stricter expression evaluation order</td>
<td><a href="http://wg21.link/p0145r3">P0145R3</a></td>
<td class="none" align="center" rowspan=2>No</td>
<td class="svn" align="center" rowspan=2>SVN <a href="#p0145">(10)</a></td>
</tr>
<tr>
<td><a href="http://wg21.link/p0400r0">P0400R0</a></td>
@ -741,6 +741,14 @@ In Clang 3.7, a warning is emitted for all cases that would change meaning.
</span><br>
<span id="p0136">(9): This is the resolution to a Defect Report, so is applied
to all language versions supporting inheriting constructors.
</span><br>
<span id="p0145">(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
<tt>operator&lt;&lt;</tt>, <tt>operator&gt;&gt;</tt>, and <tt>operator-&gt;*</tt>
functions are evaluated right-to-left under the MS ABI when called using operator
syntax, not left-to-right as P0145R3 requires.
</span>
</p>