Skip to content

[clang][bytecode] Override InConstantContext flag for immediate calls #109967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

tbaederr
Copy link
Contributor

And fix the diagnostics for __builtin_is_constant_evaluated(). We can be in a non-constant context, but calling an immediate function always makes the context constant for the duration of that call.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 25, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 25, 2024

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

And fix the diagnostics for __builtin_is_constant_evaluated(). We can be in a non-constant context, but calling an immediate function always makes the context constant for the duration of that call.


Full diff: https://github.com/llvm/llvm-project/pull/109967.diff

5 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Interp.cpp (+1)
  • (modified) clang/lib/AST/ByteCode/InterpBuiltin.cpp (+8-2)
  • (modified) clang/lib/AST/ByteCode/InterpState.cpp (+7)
  • (modified) clang/lib/AST/ByteCode/InterpState.h (+23-1)
  • (modified) clang/test/CodeGenCXX/cxx2a-consteval.cpp (+8)
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index b9c85626ffa990..2f4a05a85753c0 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1136,6 +1136,7 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
   InterpFrame *FrameBefore = S.Current;
   S.Current = NewFrame.get();
 
+  InterpStateCCOverride CCOverride(S, Func->getDecl()->isImmediateFunction());
   APValue CallResult;
   // Note that we cannot assert(CallResult.hasValue()) here since
   // Ret() above only sets the APValue if the curent frame doesn't
diff --git a/clang/lib/AST/ByteCode/InterpBuiltin.cpp b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
index 68710f67be2003..0a90fb365cc5ad 100644
--- a/clang/lib/AST/ByteCode/InterpBuiltin.cpp
+++ b/clang/lib/AST/ByteCode/InterpBuiltin.cpp
@@ -136,15 +136,21 @@ static bool retPrimValue(InterpState &S, CodePtr OpPC, APValue &Result,
 static bool interp__builtin_is_constant_evaluated(InterpState &S, CodePtr OpPC,
                                                   const InterpFrame *Frame,
                                                   const CallExpr *Call) {
+  unsigned Depth = S.Current->getDepth();
+  auto isStdCall = [](const FunctionDecl *F) -> bool {
+    return F && F->isInStdNamespace() && F->getIdentifier() &&
+           F->getIdentifier()->isStr("is_constant_evaluated");
+  };
+  const InterpFrame *Caller = Frame->Caller;
   // The current frame is the one for __builtin_is_constant_evaluated.
   // The one above that, potentially the one for std::is_constant_evaluated().
   if (S.inConstantContext() && !S.checkingPotentialConstantExpression() &&
-      Frame->Caller && S.getEvalStatus().Diag) {
+      S.getEvalStatus().Diag &&
+      (Depth == 1 || (Depth == 2 && isStdCall(Caller->getCallee())))) {
     auto isStdCall = [](const FunctionDecl *F) -> bool {
       return F && F->isInStdNamespace() && F->getIdentifier() &&
              F->getIdentifier()->isStr("is_constant_evaluated");
     };
-    const InterpFrame *Caller = Frame->Caller;
 
     if (Caller->Caller && isStdCall(Caller->getCallee())) {
       const Expr *E = Caller->Caller->getExpr(Caller->getRetPC());
diff --git a/clang/lib/AST/ByteCode/InterpState.cpp b/clang/lib/AST/ByteCode/InterpState.cpp
index 4ea05305540ee1..287c3bd3bca3a5 100644
--- a/clang/lib/AST/ByteCode/InterpState.cpp
+++ b/clang/lib/AST/ByteCode/InterpState.cpp
@@ -19,6 +19,13 @@ InterpState::InterpState(State &Parent, Program &P, InterpStack &Stk,
                          Context &Ctx, SourceMapper *M)
     : Parent(Parent), M(M), P(P), Stk(Stk), Ctx(Ctx), Current(nullptr) {}
 
+bool InterpState::inConstantContext() const {
+  if (ConstantContextOverride)
+    return *ConstantContextOverride;
+
+  return Parent.InConstantContext;
+}
+
 InterpState::~InterpState() {
   while (Current) {
     InterpFrame *Next = Current->Caller;
diff --git a/clang/lib/AST/ByteCode/InterpState.h b/clang/lib/AST/ByteCode/InterpState.h
index 4b7371450cc98e..2a1311c86a2f2a 100644
--- a/clang/lib/AST/ByteCode/InterpState.h
+++ b/clang/lib/AST/ByteCode/InterpState.h
@@ -77,7 +77,7 @@ class InterpState final : public State, public SourceMapper {
   bool noteUndefinedBehavior() override {
     return Parent.noteUndefinedBehavior();
   }
-  bool inConstantContext() const { return Parent.InConstantContext; }
+  bool inConstantContext() const;
   bool hasActiveDiagnostic() override { return Parent.hasActiveDiagnostic(); }
   void setActiveDiagnostic(bool Flag) override {
     Parent.setActiveDiagnostic(Flag);
@@ -116,6 +116,7 @@ class InterpState final : public State, public SourceMapper {
 
 private:
   friend class EvaluationResult;
+  friend class InterpStateCCOverride;
   /// AST Walker state.
   State &Parent;
   /// Dead block chain.
@@ -124,6 +125,7 @@ class InterpState final : public State, public SourceMapper {
   SourceMapper *M;
   /// Allocator used for dynamic allocations performed via the program.
   DynamicAllocator Alloc;
+  std::optional<bool> ConstantContextOverride;
 
 public:
   /// Reference to the module containing all bytecode.
@@ -144,6 +146,26 @@ class InterpState final : public State, public SourceMapper {
       SeenGlobalTemporaries;
 };
 
+class InterpStateCCOverride final {
+public:
+  InterpStateCCOverride(InterpState &Ctx, bool Value)
+      : Ctx(Ctx), OldCC(Ctx.ConstantContextOverride) {
+    // We only override this if the new value is true.
+    Enabled = Value;
+    if (Enabled)
+      Ctx.ConstantContextOverride = Value;
+  }
+  ~InterpStateCCOverride() {
+    if (Enabled)
+      Ctx.ConstantContextOverride = OldCC;
+  }
+
+private:
+  bool Enabled;
+  InterpState &Ctx;
+  std::optional<bool> OldCC;
+};
+
 } // namespace interp
 } // namespace clang
 
diff --git a/clang/test/CodeGenCXX/cxx2a-consteval.cpp b/clang/test/CodeGenCXX/cxx2a-consteval.cpp
index a58a09554699db..bfeabc946da413 100644
--- a/clang/test/CodeGenCXX/cxx2a-consteval.cpp
+++ b/clang/test/CodeGenCXX/cxx2a-consteval.cpp
@@ -6,6 +6,14 @@
 // RUN: %clang_cc1 -emit-llvm %s -Dconsteval="" -std=c++2a -triple x86_64-unknown-linux-gnu -o %t.ll
 // RUN: FileCheck -check-prefix=EXPR -input-file=%t.ll %s
 
+// RUN: %clang_cc1 -emit-llvm %s -std=c++2a -triple x86_64-unknown-linux-gnu -o %t.ll -fexperimental-new-constant-interpreter
+// RUN: FileCheck -check-prefix=EVAL -input-file=%t.ll %s
+// RUN: FileCheck -check-prefix=EVAL-STATIC -input-file=%t.ll %s
+// RUN: FileCheck -check-prefix=EVAL-FN -input-file=%t.ll %s
+//
+// RUN: %clang_cc1 -emit-llvm %s -Dconsteval="" -std=c++2a -triple x86_64-unknown-linux-gnu -o %t.ll -fexperimental-new-constant-interpreter
+// RUN: FileCheck -check-prefix=EXPR -input-file=%t.ll %s
+
 // there is two version of symbol checks to ensure
 // that the symbol we are looking for are correct
 // EVAL-NOT: @__cxx_global_var_init()

And fix the diagnostics for __builtin_is_constant_evaluated().
We can be in a non-constant context, but calling an immediate function
always makes the context constant for the duration of that call.
@tbaederr tbaederr merged commit a024a0c into llvm:main Sep 25, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants