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

[c++17] Implement P0145R3 during constant evaluation. 

Ensure that we evaluate assignment and compound-assignment
right-to-left, and array subscripting left-to-right.

Fixes PR47724.
This commit is contained in:
Richard Smith 2020-10-05 17:52:23 -07:00
parent ebf6fd633e
commit ded79be635
3 changed files with 170 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

97
clang/lib/AST/ExprConstant.cpp
View File

@ -1856,8 +1856,12 @@ void CallStackFrame::describe(raw_ostream &Out) {
Out << ", ";
const ParmVarDecl *Param = *I;
const APValue &Arg = Arguments[ArgIndex];
Arg.printPretty(Out, Info.Ctx, Param->getType());
if (Arguments) {
const APValue &Arg = Arguments[ArgIndex];
Arg.printPretty(Out, Info.Ctx, Param->getType());
} else {
Out << "<...>";
}
if (ArgIndex == 0 && IsMemberCall)
Out << "->" << *Callee << '(';
@ -5792,6 +5796,8 @@ typedef SmallVector<APValue, 8> ArgVector;
/// EvaluateArgs - Evaluate the arguments to a function call.
static bool EvaluateArgs(ArrayRef<const Expr *> Args, ArgVector &ArgValues,
EvalInfo &Info, const FunctionDecl *Callee) {
ArgValues.resize(Args.size());
bool Success = true;
llvm::SmallBitVector ForbiddenNullArgs;
if (Callee->hasAttr<NonNullAttr>()) {
@ -5809,8 +5815,6 @@ static bool EvaluateArgs(ArrayRef<const Expr *> Args, ArgVector &ArgValues,
}
}
}
// FIXME: This is the wrong evaluation order for an assignment operator
// called via operator syntax.
for (unsigned Idx = 0; Idx < Args.size(); Idx++) {
if (!Evaluate(ArgValues[Idx], Info, Args[Idx])) {
// If we're checking for a potential constant expression, evaluate all
@ -5834,17 +5838,13 @@ static bool EvaluateArgs(ArrayRef<const Expr *> Args, ArgVector &ArgValues,
/// Evaluate a function call.
static bool HandleFunctionCall(SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
ArrayRef<const Expr*> Args, const Stmt *Body,
EvalInfo &Info, APValue &Result,
const LValue *ResultSlot) {
ArgVector ArgValues(Args.size());
if (!EvaluateArgs(Args, ArgValues, Info, Callee))
return false;
ArrayRef<const Expr *> Args, APValue *ArgValues,
const Stmt *Body, EvalInfo &Info,
APValue &Result, const LValue *ResultSlot) {
if (!Info.CheckCallLimit(CallLoc))
return false;
CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues.data());
CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues);
// For a trivial copy or move assignment, perform an APValue copy. This is
// essential for unions, where the operations performed by the assignment
@ -7293,6 +7293,8 @@ public:
auto Args = llvm::makeArrayRef(E->getArgs(), E->getNumArgs());
bool HasQualifier = false;
ArgVector ArgValues;
// Extract function decl and 'this' pointer from the callee.
if (CalleeType->isSpecificBuiltinType(BuiltinType::BoundMember)) {
const CXXMethodDecl *Member = nullptr;
@ -7341,6 +7343,22 @@ public:
return Error(E);
}
// For an (overloaded) assignment expression, evaluate the RHS before the
// LHS.
auto *OCE = dyn_cast<CXXOperatorCallExpr>(E);
if (OCE && OCE->isAssignmentOp()) {
assert(Args.size() == 2 && "wrong number of arguments in assignment");
if (isa<CXXMethodDecl>(FD)) {
// Args[0] is the object argument.
if (!EvaluateArgs({Args[1]}, ArgValues, Info, FD))
return false;
} else {
if (!EvaluateArgs({Args[1], Args[0]}, ArgValues, Info, FD))
return false;
std::swap(ArgValues[0], ArgValues[1]);
}
}
// Overloaded operator calls to member functions are represented as normal
// calls with '*this' as the first argument.
const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD);
@ -7403,6 +7421,11 @@ public:
} else
return Error(E);
// Evaluate the arguments now if we've not already done so.
if (ArgValues.empty() && !Args.empty() &&
!EvaluateArgs(Args, ArgValues, Info, FD))
return false;
SmallVector<QualType, 4> CovariantAdjustmentPath;
if (This) {
auto *NamedMember = dyn_cast<CXXMethodDecl>(FD);
@ -7424,6 +7447,7 @@ public:
// Destructor calls are different enough that they have their own codepath.
if (auto *DD = dyn_cast<CXXDestructorDecl>(FD)) {
assert(This && "no 'this' pointer for destructor call");
assert(ArgValues.empty() && "unexpected destructor arguments");
return HandleDestruction(Info, E, *This,
Info.Ctx.getRecordType(DD->getParent()));
}
@ -7432,8 +7456,8 @@ public:
Stmt *Body = FD->getBody(Definition);
if (!CheckConstexprFunction(Info, E->getExprLoc(), FD, Definition, Body) ||
!HandleFunctionCall(E->getExprLoc(), Definition, This, Args, Body, Info,
Result, ResultSlot))
!HandleFunctionCall(E->getExprLoc(), Definition, This, Args,
ArgValues.data(), Body, Info, Result, ResultSlot))
return false;
if (!CovariantAdjustmentPath.empty() &&
@ -8071,16 +8095,19 @@ bool LValueExprEvaluator::VisitArraySubscriptExpr(const ArraySubscriptExpr *E) {
if (E->getBase()->getType()->isVectorType())
return Error(E);
bool Success = true;
if (!evaluatePointer(E->getBase(), Result)) {
if (!Info.noteFailure())
return false;
Success = false;
}
APSInt Index;
if (!EvaluateInteger(E->getIdx(), Index, Info))
return false;
bool Success = true;
// C++17's rules require us to evaluate the LHS first, regardless of which
// side is the base.
for (const Expr *SubExpr : {E->getLHS(), E->getRHS()}) {
if (SubExpr == E->getBase() ? !evaluatePointer(SubExpr, Result)
: !EvaluateInteger(SubExpr, Index, Info)) {
if (!Info.noteFailure())
return false;
Success = false;
}
}
return Success &&
HandleLValueArrayAdjustment(Info, E, Result, E->getType(), Index);
@ -8125,7 +8152,10 @@ bool LValueExprEvaluator::VisitCompoundAssignOperator(
if (!Info.getLangOpts().CPlusPlus14 && !Info.keepEvaluatingAfterFailure())
return Error(CAO);
// C++17 onwards require that we evaluate the RHS first.
APValue RHS;
if (!Evaluate(RHS, this->Info, CAO->getRHS()))
return false;
// The overall lvalue result is the result of evaluating the LHS.
if (!this->Visit(CAO->getLHS())) {
@ -8134,9 +8164,6 @@ bool LValueExprEvaluator::VisitCompoundAssignOperator(
return false;
}
if (!Evaluate(RHS, this->Info, CAO->getRHS()))
return false;
return handleCompoundAssignment(
this->Info, CAO,
Result, CAO->getLHS()->getType(), CAO->getComputationLHSType(),
@ -8147,7 +8174,10 @@ bool LValueExprEvaluator::VisitBinAssign(const BinaryOperator *E) {
if (!Info.getLangOpts().CPlusPlus14 && !Info.keepEvaluatingAfterFailure())
return Error(E);
// C++17 onwards require that we evaluate the RHS first.
APValue NewVal;
if (!Evaluate(NewVal, this->Info, E->getRHS()))
return false;
if (!this->Visit(E->getLHS())) {
if (Info.noteFailure())
@ -8155,9 +8185,6 @@ bool LValueExprEvaluator::VisitBinAssign(const BinaryOperator *E) {
return false;
}
if (!Evaluate(NewVal, this->Info, E->getRHS()))
return false;
if (Info.getLangOpts().CPlusPlus20 &&
!HandleUnionActiveMemberChange(Info, E->getLHS(), Result))
return false;
@ -15270,7 +15297,8 @@ bool Expr::isPotentialConstantExpr(const FunctionDecl *FD,
} else {
SourceLocation Loc = FD->getLocation();
HandleFunctionCall(Loc, FD, (MD && MD->isInstance()) ? &This : nullptr,
Args, FD->getBody(), Info, Scratch, nullptr);
Args, /*ArgValues*/ nullptr, FD->getBody(), Info,
Scratch, nullptr);
}
return Diags.empty();
@ -15292,13 +15320,8 @@ bool Expr::isPotentialConstantExprUnevaluated(Expr *E,
Info.CheckingPotentialConstantExpression = true;
// Fabricate a call stack frame to give the arguments a plausible cover story.
ArrayRef<const Expr*> Args;
ArgVector ArgValues(0);
bool Success = EvaluateArgs(Args, ArgValues, Info, FD);
(void)Success;
assert(Success &&
"Failed to set up arguments for potential constant evaluation");
CallStackFrame Frame(Info, SourceLocation(), FD, nullptr, ArgValues.data());
CallStackFrame Frame(Info, SourceLocation(), FD, /*This*/ nullptr,
/*ArgValues*/ nullptr);
APValue ResultScratch;
Evaluate(ResultScratch, Info, E);

109
clang/test/SemaCXX/constant-expression-cxx1z.cpp
View File

@ -59,3 +59,112 @@ void test() {
else if constexpr (v<int>) {}
}
}
// Check that assignment operators evaluate their operands right-to-left.
namespace EvalOrder {
template<typename T> struct lvalue {
T t;
constexpr T &get() { return t; }
};
struct UserDefined {
int n = 0;
constexpr UserDefined &operator=(const UserDefined&) { return *this; }
constexpr UserDefined &operator+=(const UserDefined&) { return *this; }
constexpr void operator<<(const UserDefined&) const {}
constexpr void operator>>(const UserDefined&) const {}
constexpr void operator+(const UserDefined&) const {}
constexpr void operator[](int) const {}
};
constexpr UserDefined ud;
struct NonMember {};
constexpr void operator+=(NonMember, NonMember) {}
constexpr void operator<<(NonMember, NonMember) {}
constexpr void operator>>(NonMember, NonMember) {}
constexpr void operator+(NonMember, NonMember) {}
constexpr NonMember nm;
constexpr void f(...) {}
// Helper to ensure that 'a' is evaluated before 'b'.
struct seq_checker {
bool done_a = false;
bool done_b = false;
template <typename T> constexpr T &&a(T &&v) {
done_a = true;
return (T &&)v;
}
template <typename T> constexpr T &&b(T &&v) {
if (!done_a)
throw "wrong";
done_b = true;
return (T &&)v;
}
constexpr bool ok() { return done_a && done_b; }
};
// SEQ(expr), where part of the expression is tagged A(...) and part is
// tagged B(...), checks that A is evaluated before B.
#define A sc.a
#define B sc.b
#define SEQ(...) static_assert([](seq_checker sc) { void(__VA_ARGS__); return sc.ok(); }({}))
// Longstanding sequencing rules.
SEQ((A(1), B(2)));
SEQ((A(true) ? B(2) : throw "huh?"));
SEQ((A(false) ? throw "huh?" : B(2)));
SEQ(A(true) && B(true));
SEQ(A(false) || B(true));
// From P0145R3:
// Rules 1 and 2 have no effect ('b' is not an expression).
// Rule 3: a->*b
SEQ(A(ud).*B(&UserDefined::n));
SEQ(A(&ud)->*B(&UserDefined::n));
// Rule 4: a(b1, b2, b3)
SEQ(A(f)(B(1), B(2), B(3)));
// Rule 5: b = a, b @= a
SEQ(B(lvalue<int>().get()) = A(0));
SEQ(B(lvalue<UserDefined>().get()) = A(ud));
SEQ(B(lvalue<int>().get()) += A(0));
SEQ(B(lvalue<UserDefined>().get()) += A(ud));
SEQ(B(lvalue<NonMember>().get()) += A(nm));
// Rule 6: a[b]
constexpr int arr[3] = {};
SEQ(A(arr)[B(0)]);
SEQ(A(+arr)[B(0)]);
SEQ(A(0)[B(arr)]);
SEQ(A(0)[B(+arr)]);
SEQ(A(ud)[B(0)]);
// Rule 7: a << b
SEQ(A(1) << B(2));
SEQ(A(ud) << B(ud));
SEQ(A(nm) << B(nm));
// Rule 8: a >> b
SEQ(A(1) >> B(2));
SEQ(A(ud) >> B(ud));
SEQ(A(nm) >> B(nm));
// No particular order of evaluation is specified in other cases, but we in
// practice evaluate left-to-right.
// FIXME: Technically we're expected to check for undefined behavior due to
// unsequenced read and modification and treat it as non-constant due to UB.
SEQ(A(1) + B(2));
SEQ(A(ud) + B(ud));
SEQ(A(nm) + B(nm));
SEQ(f(A(1), B(2)));
#undef SEQ
#undef A
#undef B
}

1
clang/www/cxx_status.html
View File

@ -807,6 +807,7 @@ left to right in the callee. As a result, function parameters in calls to
<tt>operator&amp;&amp;</tt>, <tt>operator||</tt>, and <tt>operator,</tt>
functions using expression syntax are no longer guaranteed to be destroyed in
reverse construction order in that ABI.
This is not fully supported during constant expression evaluation until Clang 12.
</span><br>
<span id="p0522">(10): Despite being the resolution to a Defect Report, this
feature is disabled by default in all language versions, and can be enabled