Skip to content

[clang][bytecode] Enter a non-constant context when revisiting #136104

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
Apr 17, 2025

Conversation

tbaederr
Copy link
Contributor

Otherwise, things like __builtin_is_constant_evaluated() return the wrong value.

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

llvmbot commented Apr 17, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Otherwise, things like __builtin_is_constant_evaluated() return the wrong value.


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

5 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Compiler.cpp (+5)
  • (modified) clang/lib/AST/ByteCode/Interp.h (+9)
  • (modified) clang/lib/AST/ByteCode/InterpState.h (+1-1)
  • (modified) clang/lib/AST/ByteCode/Opcodes.td (+3)
  • (modified) clang/test/AST/ByteCode/builtin-constant-p.cpp (+8)
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index 157e306e5cdb3..bdff40c57b0e1 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -6466,8 +6466,13 @@ bool Compiler<Emitter>::visitDeclRef(const ValueDecl *D, const Expr *E) {
 
   // In case we need to re-visit a declaration.
   auto revisit = [&](const VarDecl *VD) -> bool {
+    if (!this->emitPushCC(E))
+      return false;
     auto VarState = this->visitDecl(VD, /*IsConstexprUnknown=*/true);
 
+    if (!this->emitPopCC(E))
+      return false;
+
     if (VarState.notCreated())
       return true;
     if (!VarState)
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index bd58c2a88e9d9..bca87fc937f9f 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2848,6 +2848,15 @@ inline bool EndSpeculation(InterpState &S, CodePtr OpPC) {
   return true;
 }
 
+inline bool PushCC(InterpState &S, CodePtr OpPC) {
+  S.ConstantContextOverride = false;
+  return true;
+}
+inline bool PopCC(InterpState &S, CodePtr OpPC) {
+  S.ConstantContextOverride = std::nullopt;
+  return true;
+}
+
 /// Do nothing and just abort execution.
 inline bool Error(InterpState &S, CodePtr OpPC) { return false; }
 
diff --git a/clang/lib/AST/ByteCode/InterpState.h b/clang/lib/AST/ByteCode/InterpState.h
index 74001b80d9c00..528c1a24e7b05 100644
--- a/clang/lib/AST/ByteCode/InterpState.h
+++ b/clang/lib/AST/ByteCode/InterpState.h
@@ -127,7 +127,6 @@ 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.
@@ -147,6 +146,7 @@ class InterpState final : public State, public SourceMapper {
   /// Things needed to do speculative execution.
   SmallVectorImpl<PartialDiagnosticAt> *PrevDiags = nullptr;
   unsigned SpeculationDepth = 0;
+  std::optional<bool> ConstantContextOverride;
 
   llvm::SmallVector<
       std::pair<const Expr *, const LifetimeExtendedTemporaryDecl *>>
diff --git a/clang/lib/AST/ByteCode/Opcodes.td b/clang/lib/AST/ByteCode/Opcodes.td
index 5a9079fea0846..01e4aefdaffe0 100644
--- a/clang/lib/AST/ByteCode/Opcodes.td
+++ b/clang/lib/AST/ByteCode/Opcodes.td
@@ -872,3 +872,6 @@ def GetTypeidPtr : Opcode { let Args = [ArgTypePtr]; }
 def DiagTypeid : Opcode;
 
 def CheckDestruction : Opcode;
+
+def PushCC : Opcode;
+def PopCC : Opcode;
diff --git a/clang/test/AST/ByteCode/builtin-constant-p.cpp b/clang/test/AST/ByteCode/builtin-constant-p.cpp
index ed9e606ed16aa..f5b16761bfdc9 100644
--- a/clang/test/AST/ByteCode/builtin-constant-p.cpp
+++ b/clang/test/AST/ByteCode/builtin-constant-p.cpp
@@ -121,3 +121,11 @@ constexpr int mutate6(bool mutate) {
 static_assert(mutate6(false) == 11);
 static_assert(mutate6(true) == 21); // ref-error {{static assertion failed}} \
                                     // ref-note {{evaluates to '10 == 21'}}
+
+#define fold(x) (__builtin_constant_p(x) ? (x) : (x))
+void g() {
+  /// f will be revisited when evaluating the static_assert, since it's
+  /// a local variable. But it should be visited in a non-constant context.
+  const float f = __builtin_is_constant_evaluated();
+  static_assert(fold(f == 0.0f));
+}

Otherwise, things like __builtin_is_constant_evaluated() return
the wrong value.
@tbaederr tbaederr merged commit 90ddb54 into llvm:main Apr 17, 2025
9 of 11 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