[clang] f584f04 - [ConstExprPreter] Removed the flag forcing the use of the interpreter
Nandor Licker via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 27 12:13:43 PST 2019
Author: Nandor Licker
Date: 2019-11-27T20:07:19Z
New Revision: f584f04dab69ab15c8942753a145f0c6e7693bcc
URL: https://github.com/llvm/llvm-project/commit/f584f04dab69ab15c8942753a145f0c6e7693bcc
DIFF: https://github.com/llvm/llvm-project/commit/f584f04dab69ab15c8942753a145f0c6e7693bcc.diff
LOG: [ConstExprPreter] Removed the flag forcing the use of the interpreter
Summary:
Removed the ```-fforce-experimental-new-constant-interpreter flag```, leaving
only the ```-fexperimental-new-constant-interpreter``` one. The interpreter
now always emits an error on an unsupported feature.
Allowing the interpreter to bail out would require a mapping from APValue to
interpreter memory, which will not be necessary in the final version. It is
more sensible to always emit an error if the interpreter fails.
Reviewers: jfb, Bigcheese, rsmith, dexonsmith
Subscribers: cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D70071
Added:
Modified:
clang/docs/ConstantInterpreter.rst
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/lib/AST/ExprConstant.cpp
clang/lib/AST/Interp/Context.cpp
clang/lib/AST/Interp/Context.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/AST/Interp/cond.cpp
Removed:
################################################################################
diff --git a/clang/docs/ConstantInterpreter.rst b/clang/docs/ConstantInterpreter.rst
index d4fb8f6f34aa..a86161c8fa01 100644
--- a/clang/docs/ConstantInterpreter.rst
+++ b/clang/docs/ConstantInterpreter.rst
@@ -10,8 +10,7 @@ Introduction
The constexpr interpreter aims to replace the existing tree evaluator in clang, improving performance on constructs which are executed inefficiently by the evaluator. The interpreter is activated using the following flags:
-* ``-fexperimental-new-constant-interpreter`` enables the interpreter, falling back to the evaluator for unsupported features
-* ``-fforce-experimental-new-constant-interpreter`` forces the use of the interpreter, bailing out if an unsupported feature is encountered
+* ``-fexperimental-new-constant-interpreter`` enables the interpreter, emitting an error if an unsupported feature is encountered
Bytecode Compilation
====================
diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def
index 82bf379af909..68d6ee1dce42 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -297,8 +297,6 @@ BENIGN_LANGOPT(ConstexprStepLimit, 32, 1048576,
"maximum constexpr evaluation steps")
BENIGN_LANGOPT(EnableNewConstInterp, 1, 0,
"enable the experimental new constant interpreter")
-BENIGN_LANGOPT(ForceNewConstInterp, 1, 0,
- "force the use of the experimental new constant interpreter")
BENIGN_LANGOPT(BracketDepth, 32, 256,
"maximum bracket nesting depth")
BENIGN_LANGOPT(NumLargeByValueCopy, 32, 0,
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 2d501c09c762..daba98a39dab 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -850,8 +850,6 @@ def fconstexpr_depth_EQ : Joined<["-"], "fconstexpr-depth=">, Group<f_Group>;
def fconstexpr_steps_EQ : Joined<["-"], "fconstexpr-steps=">, Group<f_Group>;
def fexperimental_new_constant_interpreter : Flag<["-"], "fexperimental-new-constant-interpreter">, Group<f_Group>,
HelpText<"Enable the experimental new constant interpreter">, Flags<[CC1Option]>;
-def fforce_experimental_new_constant_interpreter : Flag<["-"], "fforce-experimental-new-constant-interpreter">, Group<f_Group>,
- HelpText<"Force the use of the experimental new constant interpreter, failing on missing features">, Flags<[CC1Option]>;
def fconstexpr_backtrace_limit_EQ : Joined<["-"], "fconstexpr-backtrace-limit=">,
Group<f_Group>;
def fno_crash_diagnostics : Flag<["-"], "fno-crash-diagnostics">, Group<f_clang_Group>, Flags<[NoArgumentUnused, CoreOption]>,
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index eec9bbdaef80..df80cb4f9440 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -763,11 +763,8 @@ namespace {
/// we will evaluate.
unsigned StepsLeft;
- /// Force the use of the experimental new constant interpreter, bailing out
- /// with an error if a feature is not supported.
- bool ForceNewConstInterp;
-
- /// Enable the experimental new constant interpreter.
+ /// Enable the experimental new constant interpreter. If an expression is
+ /// not supported by the interpreter, an error is triggered.
bool EnableNewConstInterp;
/// BottomFrame - The frame in which evaluation started. This must be
@@ -922,9 +919,7 @@ namespace {
: Ctx(const_cast<ASTContext &>(C)), EvalStatus(S), CurrentCall(nullptr),
CallStackDepth(0), NextCallIndex(1),
StepsLeft(C.getLangOpts().ConstexprStepLimit),
- ForceNewConstInterp(C.getLangOpts().ForceNewConstInterp),
- EnableNewConstInterp(ForceNewConstInterp ||
- C.getLangOpts().EnableNewConstInterp),
+ EnableNewConstInterp(C.getLangOpts().EnableNewConstInterp),
BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
EvaluatingDecl((const ValueDecl *)nullptr),
EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
@@ -13400,32 +13395,25 @@ static bool EvaluateInPlace(APValue &Result, EvalInfo &Info, const LValue &This,
/// EvaluateAsRValue - Try to evaluate this expression, performing an implicit
/// lvalue-to-rvalue cast if it is an lvalue.
static bool EvaluateAsRValue(EvalInfo &Info, const Expr *E, APValue &Result) {
- if (Info.EnableNewConstInterp) {
- auto &InterpCtx = Info.Ctx.getInterpContext();
- switch (InterpCtx.evaluateAsRValue(Info, E, Result)) {
- case interp::InterpResult::Success:
- return true;
- case interp::InterpResult::Fail:
+ if (Info.EnableNewConstInterp) {
+ if (!Info.Ctx.getInterpContext().evaluateAsRValue(Info, E, Result))
+ return false;
+ } else {
+ if (E->getType().isNull())
return false;
- case interp::InterpResult::Bail:
- break;
- }
- }
-
- if (E->getType().isNull())
- return false;
-
- if (!CheckLiteralType(Info, E))
- return false;
- if (!::Evaluate(Result, Info, E))
- return false;
+ if (!CheckLiteralType(Info, E))
+ return false;
- if (E->isGLValue()) {
- LValue LV;
- LV.setFrom(Info.Ctx, Result);
- if (!handleLValueToRValueConversion(Info, E, E->getType(), LV, Result))
+ if (!::Evaluate(Result, Info, E))
return false;
+
+ if (E->isGLValue()) {
+ LValue LV;
+ LV.setFrom(Info.Ctx, Result);
+ if (!handleLValueToRValueConversion(Info, E, E->getType(), LV, Result))
+ return false;
+ }
}
// Check this core constant expression is a constant expression.
@@ -13637,46 +13625,36 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
if (Info.EnableNewConstInterp) {
auto &InterpCtx = const_cast<ASTContext &>(Ctx).getInterpContext();
- switch (InterpCtx.evaluateAsInitializer(Info, VD, Value)) {
- case interp::InterpResult::Fail:
- // Bail out if an error was encountered.
- return false;
- case interp::InterpResult::Success:
- // Evaluation succeeded and value was set.
- return CheckConstantExpression(Info, DeclLoc, DeclTy, Value);
- case interp::InterpResult::Bail:
- // Evaluate the value again for the tree evaluator to use.
- break;
+ if (!InterpCtx.evaluateAsInitializer(Info, VD, Value))
+ return false;
+ } else {
+ LValue LVal;
+ LVal.set(VD);
+
+ // C++11 [basic.start.init]p2:
+ // Variables with static storage duration or thread storage duration shall
+ // be zero-initialized before any other initialization takes place.
+ // This behavior is not present in C.
+ if (Ctx.getLangOpts().CPlusPlus && !VD->hasLocalStorage() &&
+ !DeclTy->isReferenceType()) {
+ ImplicitValueInitExpr VIE(DeclTy);
+ if (!EvaluateInPlace(Value, Info, LVal, &VIE,
+ /*AllowNonLiteralTypes=*/true))
+ return false;
}
- }
-
- LValue LVal;
- LVal.set(VD);
- // C++11 [basic.start.init]p2:
- // Variables with static storage duration or thread storage duration shall be
- // zero-initialized before any other initialization takes place.
- // This behavior is not present in C.
- if (Ctx.getLangOpts().CPlusPlus && !VD->hasLocalStorage() &&
- !DeclTy->isReferenceType()) {
- ImplicitValueInitExpr VIE(DeclTy);
- if (!EvaluateInPlace(Value, Info, LVal, &VIE,
- /*AllowNonLiteralTypes=*/true))
+ if (!EvaluateInPlace(Value, Info, LVal, this,
+ /*AllowNonLiteralTypes=*/true) ||
+ EStatus.HasSideEffects)
return false;
- }
-
- if (!EvaluateInPlace(Value, Info, LVal, this,
- /*AllowNonLiteralTypes=*/true) ||
- EStatus.HasSideEffects)
- return false;
-
- // At this point, any lifetime-extended temporaries are completely
- // initialized.
- Info.performLifetimeExtension();
- if (!Info.discardCleanups())
- llvm_unreachable("Unhandled cleanup; missing full expression marker?");
+ // At this point, any lifetime-extended temporaries are completely
+ // initialized.
+ Info.performLifetimeExtension();
+ if (!Info.discardCleanups())
+ llvm_unreachable("Unhandled cleanup; missing full expression marker?");
+ }
return CheckConstantExpression(Info, DeclLoc, DeclTy, Value) &&
CheckMemoryLeaks(Info);
}
@@ -14415,14 +14393,8 @@ bool Expr::isPotentialConstantExpr(const FunctionDecl *FD,
// The constexpr VM attempts to compile all methods to bytecode here.
if (Info.EnableNewConstInterp) {
- auto &InterpCtx = Info.Ctx.getInterpContext();
- switch (InterpCtx.isPotentialConstantExpr(Info, FD)) {
- case interp::InterpResult::Success:
- case interp::InterpResult::Fail:
- return Diags.empty();
- case interp::InterpResult::Bail:
- break;
- }
+ Info.Ctx.getInterpContext().isPotentialConstantExpr(Info, FD);
+ return Diags.empty();
}
const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD);
diff --git a/clang/lib/AST/Interp/Context.cpp b/clang/lib/AST/Interp/Context.cpp
index 4f8f7b96e7c3..e7f9ba0f010a 100644
--- a/clang/lib/AST/Interp/Context.cpp
+++ b/clang/lib/AST/Interp/Context.cpp
@@ -21,44 +21,37 @@
using namespace clang;
using namespace clang::interp;
-Context::Context(ASTContext &Ctx)
- : Ctx(Ctx), ForceInterp(getLangOpts().ForceNewConstInterp),
- P(new Program(*this)) {}
+Context::Context(ASTContext &Ctx) : Ctx(Ctx), P(new Program(*this)) {}
Context::~Context() {}
-InterpResult Context::isPotentialConstantExpr(State &Parent,
- const FunctionDecl *FD) {
+bool Context::isPotentialConstantExpr(State &Parent, const FunctionDecl *FD) {
Function *Func = P->getFunction(FD);
if (!Func) {
if (auto R = ByteCodeStmtGen<ByteCodeEmitter>(*this, *P).compileFunc(FD)) {
Func = *R;
- } else if (ForceInterp) {
+ } else {
handleAllErrors(R.takeError(), [&Parent](ByteCodeGenError &Err) {
Parent.FFDiag(Err.getLoc(), diag::err_experimental_clang_interp_failed);
});
- return InterpResult::Fail;
- } else {
- consumeError(R.takeError());
- return InterpResult::Bail;
+ return false;
}
}
if (!Func->isConstexpr())
- return InterpResult::Fail;
+ return false;
APValue Dummy;
return Run(Parent, Func, Dummy);
}
-InterpResult Context::evaluateAsRValue(State &Parent, const Expr *E,
- APValue &Result) {
+bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
ByteCodeExprGen<EvalEmitter> C(*this, *P, Parent, Stk, Result);
return Check(Parent, C.interpretExpr(E));
}
-InterpResult Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
- APValue &Result) {
+bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,
+ APValue &Result) {
ByteCodeExprGen<EvalEmitter> C(*this, *P, Parent, Stk, Result);
return Check(Parent, C.interpretDecl(VD));
}
@@ -116,33 +109,20 @@ unsigned Context::getCharBit() const {
return Ctx.getTargetInfo().getCharWidth();
}
-InterpResult Context::Run(State &Parent, Function *Func, APValue &Result) {
- InterpResult Flag;
- {
- InterpState State(Parent, *P, Stk, *this);
- State.Current = new InterpFrame(State, Func, nullptr, {}, {});
- if (Interpret(State, Result)) {
- Flag = InterpResult::Success;
- } else {
- Flag = InterpResult::Fail;
- }
- }
-
- if (Flag != InterpResult::Success)
- Stk.clear();
- return Flag;
+bool Context::Run(State &Parent, Function *Func, APValue &Result) {
+ InterpState State(Parent, *P, Stk, *this);
+ State.Current = new InterpFrame(State, Func, nullptr, {}, {});
+ if (Interpret(State, Result))
+ return true;
+ Stk.clear();
+ return false;
}
-InterpResult Context::Check(State &Parent, llvm::Expected<bool> &&R) {
- if (R) {
- return *R ? InterpResult::Success : InterpResult::Fail;
- } else if (ForceInterp) {
- handleAllErrors(R.takeError(), [&Parent](ByteCodeGenError &Err) {
- Parent.FFDiag(Err.getLoc(), diag::err_experimental_clang_interp_failed);
- });
- return InterpResult::Fail;
- } else {
- consumeError(R.takeError());
- return InterpResult::Bail;
- }
+bool Context::Check(State &Parent, llvm::Expected<bool> &&Flag) {
+ if (Flag)
+ return *Flag;
+ handleAllErrors(Flag.takeError(), [&Parent](ByteCodeGenError &Err) {
+ Parent.FFDiag(Err.getLoc(), diag::err_experimental_clang_interp_failed);
+ });
+ return false;
}
diff --git a/clang/lib/AST/Interp/Context.h b/clang/lib/AST/Interp/Context.h
index 96368b6e5f02..e4d831cbb991 100644
--- a/clang/lib/AST/Interp/Context.h
+++ b/clang/lib/AST/Interp/Context.h
@@ -34,16 +34,6 @@ class Program;
class State;
enum PrimType : unsigned;
-/// Wrapper around interpreter termination results.
-enum class InterpResult {
- /// Interpreter successfully computed a value.
- Success,
- /// Interpreter encountered an error and quit.
- Fail,
- /// Interpreter encountered an unimplemented feature, AST fallback.
- Bail,
-};
-
/// Holds all information required to evaluate constexpr code in a module.
class Context {
public:
@@ -54,15 +44,13 @@ class Context {
~Context();
/// Checks if a function is a potential constant expression.
- InterpResult isPotentialConstantExpr(State &Parent,
- const FunctionDecl *FnDecl);
+ bool isPotentialConstantExpr(State &Parent, const FunctionDecl *FnDecl);
/// Evaluates a toplevel expression as an rvalue.
- InterpResult evaluateAsRValue(State &Parent, const Expr *E, APValue &Result);
+ bool evaluateAsRValue(State &Parent, const Expr *E, APValue &Result);
/// Evaluates a toplevel initializer.
- InterpResult evaluateAsInitializer(State &Parent, const VarDecl *VD,
- APValue &Result);
+ bool evaluateAsInitializer(State &Parent, const VarDecl *VD, APValue &Result);
/// Returns the AST context.
ASTContext &getASTContext() const { return Ctx; }
@@ -78,16 +66,14 @@ class Context {
private:
/// Runs a function.
- InterpResult Run(State &Parent, Function *Func, APValue &Result);
+ bool Run(State &Parent, Function *Func, APValue &Result);
/// Checks a result fromt the interpreter.
- InterpResult Check(State &Parent, llvm::Expected<bool> &&R);
+ bool Check(State &Parent, llvm::Expected<bool> &&R);
private:
/// Current compilation context.
ASTContext &Ctx;
- /// Flag to indicate if the use of the interpreter is mandatory.
- bool ForceInterp;
/// Interpreter stack, shared across invocations.
InterpStack Stk;
/// Constexpr program.
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 03ebef550cde..26d13c714670 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4503,9 +4503,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
if (Args.hasArg(options::OPT_fexperimental_new_constant_interpreter))
CmdArgs.push_back("-fexperimental-new-constant-interpreter");
- if (Args.hasArg(options::OPT_fforce_experimental_new_constant_interpreter))
- CmdArgs.push_back("-fforce-experimental-new-constant-interpreter");
-
if (Arg *A = Args.getLastArg(options::OPT_fbracket_depth_EQ)) {
CmdArgs.push_back("-fbracket-depth");
CmdArgs.push_back(A->getValue());
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index 56f665375575..74831e78d8cb 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -2854,8 +2854,6 @@ static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
getLastArgIntValue(Args, OPT_fconstexpr_steps, 1048576, Diags);
Opts.EnableNewConstInterp =
Args.hasArg(OPT_fexperimental_new_constant_interpreter);
- Opts.ForceNewConstInterp =
- Args.hasArg(OPT_fforce_experimental_new_constant_interpreter);
Opts.BracketDepth = getLastArgIntValue(Args, OPT_fbracket_depth, 256, Diags);
Opts.DelayedTemplateParsing = Args.hasArg(OPT_fdelayed_template_parsing);
Opts.NumLargeByValueCopy =
diff --git a/clang/test/AST/Interp/cond.cpp b/clang/test/AST/Interp/cond.cpp
index 8a5a318c216d..1fc69ed333e1 100644
--- a/clang/test/AST/Interp/cond.cpp
+++ b/clang/test/AST/Interp/cond.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fforce-experimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fexperimental-new-constant-interpreter %s -verify
// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s -verify
// expected-no-diagnostics
More information about the cfe-commits
mailing list