[clang] [clang-tools-extra] [clang][bytecode] Don't evaluate bound member function expressions in new constant interpreter (PR #194851)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangd

Author: cakgok (cakgok)

<details>
<summary>Changes</summary>

**Problem:**

A crash is triggered by clangd's hover feature when using C++23 and the new bytecode interpreter, which calls `Expr::EvaluateAsRValue()` to attempt constant folding on an expression under the cursor, even when it is not a valid constant expression.

Tested versions: 22.1.3, Trunk (x86_64-pc-linux-gnu)

**How to reproduce:**
```cpp
struct S { void f(); };
void g() { S s; s.f(); }
```
Running `clangd --check=repro.cpp` 
(with `compile_flags.txt` containing `-std=c++23 -fexperimental-new-constant-interpreter`) 
will crash. 

`Assertion ItemTypes.back() == toPrimType<T>() failed.`

You can observe the same crash by hovering over STL iterators like `vec.begin()`.

**Relevant Stack Trace:**
```text
#<!-- -->8  clang::interp::InterpStack::pop<MemberPointer>()
#<!-- -->9  clang::interp::EvalEmitter::emitRet(PrimType, SourceInfo)
#<!-- -->10 clang::interp::Compiler<EvalEmitter>::visitExpr(Expr const*, bool)
#<!-- -->11 clang::interp::EvalEmitter::interpretExpr(Expr const*, bool, bool)
#<!-- -->12 clang::interp::Context::evaluateAsRValue(State&, Expr const*, APValue&)
#<!-- -->13 EvaluateAsRValue(EvalInfo&, Expr const*, APValue&)
#<!-- -->14 clang::Expr::EvaluateAsRValue(EvalResult&, ASTContext const&, bool) const
#<!-- -->15 clangd::(anon)::printExprValue(Expr const*, ASTContext const&)
#<!-- -->16 clangd::(anon)::printExprValue(SelectionTree::Node const*, ASTContext const&)
#<!-- -->17 clangd::getHover(...)
```
*Basically: `textDocument/hover` → `getHover` → `EvaluateAsRValue` → new constant interpreter → `MemberPointer` type mismatch on stack pop.*

When `Compiler<Emitter>::VisitMemberExpr()` encounters a non-static `CXXMethodDecl` member (a bound member function expression such as `s.f` in `s.f()`), it falls through to `visitDeclRef()`. This pushes a `FnPtr` onto the interpreter stack. However, the caller expects a `MemberPointer`, causing an assertion failure in `InterpStack::pop()`:

**Fix:**

* In `VisitMemberExpr()`, bail out early (`return false`) when the member is a non-static `CXXMethodDecl`, before reaching `visitDeclRef()`. This causes `EvaluateAsRValue()` to report failure gracefully rather than crashing. Bound member function expressions (`s.f`) are not valid constant expressions, so returning `false` should be semantically correct.

**Testing:**
* Added AST unit test (`EvaluateAsRValue.FailsGracefullyOnBoundMemberExpr`) that directly isolates a bound `MemberExpr` and passes it to `EvaluateAsRValue()`, asserting it returns `false` without crashing.

* Added clangd hover test (Hover.NoCrashOnBoundMemberFunctionWithNewInterpreter)
  that reproduces the original crash scenario.

* *Note:* I could not add a Lit test because I believe this is unreachable via normal `clang` invocations. `Sema` strictly catches isolated bound member functions before constant evaluation. `clangd` has a unique path to triggering this.

**Root cause:**

This is exposed by C++23 specifically due to (I think P2280R4 / P2448R2):

-  Relaxing the rules around "unknown" objects in constant evaluation, allowing `s` in `s.f()` to proceed past the base object check even though `s` is not constexpr and deferring failures to bytecode execution rather than rejecting them structurally.

@<!-- -->tbaederr


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


3 Files Affected:

- (modified) clang-tools-extra/clangd/unittests/HoverTests.cpp (+12) 
- (modified) clang/lib/AST/ByteCode/Compiler.cpp (+8) 
- (modified) clang/unittests/AST/EvaluateAsRValueTest.cpp (+51) 


``````````diff
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 7b168b0bdca60..9ce244e05a024 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -5245,6 +5245,18 @@ TEST(Hover, FunctionParameters) {
   }
 }
 
+TEST(Hover, NoCrashOnBoundMemberFunctionWithNewInterpreter) {
+  Annotations Code(R"cpp(
+    struct S { void f(); };
+    void g() { S s; s.^f(); }
+  )cpp");
+  TestTU TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs.push_back("-std=c++23");
+  TU.ExtraArgs.push_back("-fexperimental-new-constant-interpreter");
+  auto AST = TU.build();
+  getHover(AST, Code.point(), format::getLLVMStyle(), nullptr);
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index d4bbb8d3a8a3e..ca2a408c0eceb 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -2643,6 +2643,14 @@ bool Compiler<Emitter>::VisitMemberExpr(const MemberExpr *E) {
     if (!this->discard(Base) && !this->emitSideEffect(E))
       return false;
 
+    // Bound member functions (non-static CXXMethodDecls) cannot be
+    // constant-evaluated. visitDeclRef would blindly push a FnPtr,
+    // but the caller expects a MemberPointer, causing a stack mismatch.
+    if (const auto *MD = dyn_cast<CXXMethodDecl>(Member)) {
+      if (!MD->isStatic())
+        return false;
+    }
+
     return this->visitDeclRef(Member, E);
   }
 
diff --git a/clang/unittests/AST/EvaluateAsRValueTest.cpp b/clang/unittests/AST/EvaluateAsRValueTest.cpp
index 1e17330863f26..d98c1d09d5bd4 100644
--- a/clang/unittests/AST/EvaluateAsRValueTest.cpp
+++ b/clang/unittests/AST/EvaluateAsRValueTest.cpp
@@ -154,3 +154,54 @@ TEST(EvaluateAsRValue, LValueToRValueConversionWorks) {
                                       Args));
   }
 }
+
+class EvaluateBoundMemberFunctionVisitor
+    : public clang::DynamicRecursiveASTVisitor {
+public:
+  explicit EvaluateBoundMemberFunctionVisitor(clang::ASTContext &Ctx)
+      : Ctx(Ctx) {}
+
+  bool VisitMemberExpr(clang::MemberExpr *E) override {
+    if (llvm::isa<clang::CXXMethodDecl>(E->getMemberDecl())) {
+      clang::Expr::EvalResult Result;
+      bool EvalSucceeded = E->EvaluateAsRValue(Result, Ctx, true);
+      EXPECT_FALSE(EvalSucceeded);
+    }
+    return true;
+  }
+
+private:
+  clang::ASTContext &Ctx;
+};
+
+class EvaluateBoundMemberFunctionAction : public clang::ASTFrontendAction {
+public:
+  std::unique_ptr<clang::ASTConsumer>
+  CreateASTConsumer(clang::CompilerInstance &Compiler,
+                    llvm::StringRef FilePath) override {
+    return std::make_unique<Consumer>();
+  }
+
+private:
+  class Consumer : public clang::ASTConsumer {
+  public:
+    ~Consumer() override {}
+    void HandleTranslationUnit(clang::ASTContext &Ctx) override {
+      EvaluateBoundMemberFunctionVisitor Evaluator(Ctx);
+      Evaluator.TraverseDecl(Ctx.getTranslationUnitDecl());
+    }
+  };
+};
+
+TEST(EvaluateAsRValue, FailsGracefullyOnBoundMemberExpr) {
+  std::string ModesToTest[] = {"", "-fexperimental-new-constant-interpreter"};
+  for (std::string const &Mode : ModesToTest) {
+    std::vector<std::string> Args(1, Mode);
+    Args.push_back("-std=c++23");
+    ASSERT_TRUE(runToolOnCodeWithArgs(
+        std::make_unique<EvaluateBoundMemberFunctionAction>(),
+        "struct S { void f(); };\n"
+        "void g() { S s; s.f(); }\n",
+        Args));
+  }
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/194851