[ConstExprPreter] Removed the flag forcing the use of the interpreter

[nand]

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.

[nand]

ping

[Bigcheese]

lgtm

[nand]

rebase

clang/docs/ConstantInterpreter.rst

 ====================
 Constant Interpreter
 ====================
 
 .. contents::
    :local:
 
 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
 ====================
 
 Bytecode compilation is handled in ``ByteCodeStmtGen.h`` for statements and ``ByteCodeExprGen.h`` for expressions. The compiler has two different backends: one to generate bytecode for functions (``ByteCodeEmitter``) and one to directly evaluate expressions as they are compiled, without generating bytecode (``EvalEmitter``). All functions are compiled to bytecode, while toplevel expressions used in constant contexts are directly evaluated since the bytecode would never be reused. This mechanism aims to pave the way towards replacing the evaluator, improving its performance on functions and loops, while being just as fast on single-use toplevel expressions.
 
 The interpreter relies on stack-based, strongly-typed opcodes. The glue logic between the code generator, along with the enumeration and description of opcodes, can be found in ``Opcodes.td``. The opcodes are implemented as generic template methods in ``Interp.h`` and instantiated with the relevant primitive types by the interpreter loop or by the evaluating emitter.
 

clang/include/clang/Basic/LangOptions.def

 BENIGN_LANGOPT(InstantiationDepth, 32, 1024,
                "maximum template instantiation depth")
 BENIGN_LANGOPT(ConstexprCallDepth, 32, 512,
                "maximum constexpr call depth")
 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,
         "if non-zero, warn about parameter or return Warn if parameter/return value is larger in bytes than this setting. 0 is no check.")
 VALUE_LANGOPT(MSCompatibilityVersion, 32, 0, "Microsoft Visual C/C++ Version")
 ENUM_LANGOPT(VtorDispMode, MSVtorDispMode, 2, MSVtorDispMode::ForVBaseOverride,
              "How many vtordisps to insert")
 

clang/include/clang/Driver/Options.td

 def fcf_runtime_abi_EQ : Joined<["-"], "fcf-runtime-abi=">, Group<f_Group>,
     Flags<[CC1Option]>;
 def fconstant_cfstrings : Flag<["-"], "fconstant-cfstrings">, Group<f_Group>;
 def fconstant_string_class_EQ : Joined<["-"], "fconstant-string-class=">, Group<f_Group>;
 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]>,
   HelpText<"Disable auto-generation of preprocessed source files and a script for reproduction during a clang crash">;
 def fcrash_diagnostics_dir : Joined<["-"], "fcrash-diagnostics-dir=">, Group<f_clang_Group>, Flags<[NoArgumentUnused, CoreOption]>;
 def fcreate_profile : Flag<["-"], "fcreate-profile">, Group<f_Group>;
 def fcxx_exceptions: Flag<["-"], "fcxx-exceptions">, Group<f_Group>,
   HelpText<"Enable C++ exceptions">, Flags<[CC1Option]>;

clang/lib/AST/ExprConstant.cpp

public:
     /// NextCallIndex - The next call index to assign.
     unsigned NextCallIndex;
 
     /// StepsLeft - The remaining number of evaluation steps we're permitted
     /// to perform. This is essentially a limit for the number of statements
     /// 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
     /// initialized after CurrentCall and CallStackDepth.
     CallStackFrame BottomFrame;
 
     /// A stack of values whose lifetimes end at the end of some surrounding
     /// evaluation frame.
public:
     bool checkingForUndefinedBehavior() const override {
       return CheckingForUndefinedBehavior;
     }
 
     EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
         : 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),
           HasFoldFailureDiagnostic(false), InConstantContext(false),
           EvalMode(Mode) {}
 
     ~EvalInfo() {
       discardCleanups();
static bool EvaluateInPlace(APValue &Result, EvalInfo &Info, const LValue &This,
 
   // For any other type, in-place evaluation is unimportant.
   return Evaluate(Result, Info, E);
 }
 
 /// 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.Ctx.getInterpContext().evaluateAsRValue(Info, E, Result))
       return false;
-    case interp::InterpResult::Bail:
-      break;
-    }
-  }
-
+  } else {
     if (E->getType().isNull())
       return false;
 
     if (!CheckLiteralType(Info, E))
       return false;
 
     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.
   return CheckConstantExpression(Info, E->getExprLoc(), E->getType(), Result) &&
          CheckMemoryLeaks(Info);
 }
 
 static bool FastEvaluateAsRValue(const Expr *Exp, Expr::EvalResult &Result,
                                  const ASTContext &Ctx, bool &IsConst) {
bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
   Info.setEvaluatingDecl(VD, Value);
   Info.InConstantContext = true;
 
   SourceLocation DeclLoc = VD->getLocation();
   QualType DeclTy = VD->getType();
 
   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.
+    //  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;
     }
 
     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?");
-
+  }
   return CheckConstantExpression(Info, DeclLoc, DeclTy, Value) &&
          CheckMemoryLeaks(Info);
 }
 
 bool VarDecl::evaluateDestruction(
     SmallVectorImpl<PartialDiagnosticAt> &Notes) const {
   assert(getEvaluatedValue() && !getEvaluatedValue()->isAbsent() &&
          "cannot evaluate destruction of non-constant-initialized variable");
bool Expr::isPotentialConstantExpr(const FunctionDecl *FD,
   Status.Diag = &Diags;
 
   EvalInfo Info(FD->getASTContext(), Status, EvalInfo::EM_ConstantExpression);
   Info.InConstantContext = true;
   Info.CheckingPotentialConstantExpression = true;
 
   // 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);
   }
 
   const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(FD);
   const CXXRecordDecl *RD = MD ? MD->getParent()->getCanonicalDecl() : nullptr;
 
   // Fabricate an arbitrary expression on the stack and pretend that it
   // is a temporary being used as the 'this' pointer.
   LValue This;

clang/lib/AST/Interp/Context.h

 class VarDecl;
 
 namespace interp {
 class Function;
 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:
   /// Initialises the constexpr VM.
   Context(ASTContext &Ctx);
 
   /// Cleans up the constexpr VM.
   ~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; }
   /// Returns the language options.
   const LangOptions &getLangOpts() const;
   /// Returns the interpreter stack.
   InterpStack &getStack() { return Stk; }
   /// Returns CHAR_BIT.
   unsigned getCharBit() const;
 
   /// Classifies an expression.
   llvm::Optional<PrimType> classify(QualType T);
 
 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.
   std::unique_ptr<Program> P;
 };
 
 } // namespace interp
 } // namespace clang
 
 #endif

clang/lib/AST/Interp/Context.cpp

 #include "InterpStack.h"
 #include "PrimType.h"
 #include "Program.h"
 #include "clang/AST/Expr.h"
 
 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,
   ByteCodeExprGen<EvalEmitter> C(*this, *P, Parent, Stk, Result);
   return Check(Parent, C.interpretDecl(VD));
 }
 
 const LangOptions &Context::getLangOpts() const { return Ctx.getLangOpts(); }
 
 llvm::Optional<PrimType> Context::classify(QualType T) {
   if (T->isReferenceType() || T->isPointerType()) {
llvm::Optional<PrimType> Context::classify(QualType T) {
 
   return {};
 }
 
 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) {
+  if (Interpret(State, Result))
+    return true;
+  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) {
+  return false;
 }

clang/lib/Driver/ToolChains/Clang.cpp

#endif
   if (Arg *A = Args.getLastArg(options::OPT_fconstexpr_steps_EQ)) {
     CmdArgs.push_back("-fconstexpr-steps");
     CmdArgs.push_back(A->getValue());
   }
 
   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());
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_Wlarge_by_value_copy_EQ,
                                options::OPT_Wlarge_by_value_copy_def)) {
     if (A->getNumValues()) {

clang/lib/Frontend/CompilerInvocation.cpp

#include "clang/Basic/LangStandards.def"
   Opts.ArrowDepth =
       getLastArgIntValue(Args, OPT_foperator_arrow_depth, 256, Diags);
   Opts.ConstexprCallDepth =
       getLastArgIntValue(Args, OPT_fconstexpr_depth, 512, Diags);
   Opts.ConstexprStepLimit =
       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 =
       getLastArgIntValue(Args, OPT_Wlarge_by_value_copy_EQ, 0, Diags);
   Opts.MSBitfields = Args.hasArg(OPT_mms_bitfields);
   Opts.ObjCConstantStringClass =
     Args.getLastArgValue(OPT_fconstant_string_class);
   Opts.ObjCDefaultSynthProperties =

clang/test/AST/Interp/cond.cpp

-// 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
 
 constexpr int cond_then_else(int a, int b) {
   if (a < b) {
     return b - a;
   } else {
     return a - b;
   }
 }