[PATCH] D70071: [ConstExprPreter] Removed the flag forcing the use of the interpreter

[Nandor Licker via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf584f04dab69: [ConstExprPreter] Removed the flag forcing the 
use of the interpreter (authored by nand).

Changed prior to commit:
  https://reviews.llvm.org/D70071?vs=230791&id=231310#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70071/new/

https://reviews.llvm.org/D70071

Files:
  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

Index: clang/test/AST/Interp/cond.cpp
===
--- clang/test/AST/Interp/cond.cpp
+++ 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
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2854,8 +2854,6 @@
   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 =
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4503,9 +4503,6 @@
   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());
Index: clang/lib/AST/Interp/Context.h
===
--- clang/lib/AST/Interp/Context.h
+++ clang/lib/AST/Interp/Context.h
@@ -34,16 +34,6 @@
 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 @@
   ~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 @@
 
 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 &&R);
+  bool Check(State &Parent, llvm::Expected &&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.
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -21,44 +21,37 @@
 using namespace clang;
 using namespace clang::interp;
 
-Context::Context(ASTContext &Ctx)
-: Ctx(Ctx), ForceInterp(getLang

[PATCH] D70071: [ConstExprPreter] Removed the flag forcing the use of the interpreter

[Nandor Licker via Phabricator via cfe-commits]

nand updated this revision to Diff 230791.
nand added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70071/new/

https://reviews.llvm.org/D70071

Files:
  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

Index: clang/test/AST/Interp/cond.cpp
===
--- clang/test/AST/Interp/cond.cpp
+++ 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
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2854,8 +2854,6 @@
   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 =
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4484,9 +4484,6 @@
   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());
Index: clang/lib/AST/Interp/Context.h
===
--- clang/lib/AST/Interp/Context.h
+++ clang/lib/AST/Interp/Context.h
@@ -34,16 +34,6 @@
 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 @@
   ~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 @@
 
 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 &&R);
+  bool Check(State &Parent, llvm::Expected &&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.
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ 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(

[PATCH] D70071: [ConstExprPreter] Removed the flag forcing the use of the interpreter

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese accepted this revision.
Bigcheese added a comment.
This revision is now accepted and ready to land.

lgtm


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70071/new/

https://reviews.llvm.org/D70071



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70071: [ConstExprPreter] Removed the flag forcing the use of the interpreter

[Nandor Licker via Phabricator via cfe-commits]

nand added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70071/new/

https://reviews.llvm.org/D70071



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70071: [ConstExprPreter] Removed the flag forcing the use of the interpreter

[Nandor Licker via Phabricator via cfe-commits]

nand created this revision.
nand added reviewers: jfb, Bigcheese, rsmith, dexonsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
nand edited the summary of this revision.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70071

Files:
  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

Index: clang/test/AST/Interp/cond.cpp
===
--- clang/test/AST/Interp/cond.cpp
+++ 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
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -2836,8 +2836,6 @@
   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 =
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4428,9 +4428,6 @@
   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());
Index: clang/lib/AST/Interp/Context.h
===
--- clang/lib/AST/Interp/Context.h
+++ clang/lib/AST/Interp/Context.h
@@ -34,16 +34,6 @@
 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 @@
   ~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 @@
 
 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 &&R);
+  bool Check(State &Parent, llvm::Expected &&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.
Index: clang/lib/AST/Interp/Context.cpp