[clang] Use new interpreter in EvaluateAsConstantExpr if requested

[tbaederr]

EvaluateAsConstantExpr() uses ::EvaluateInPlace() directly, which does not use the new interpreter if requested. Do it here, which is the same pattern we use in EvaluateAsInitializer.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

EvaluateAsConstantExpr() uses ::EvaluateInPlace() directly, which does not use the new interpreter if requested. Do it here, which is the same pattern we use in EvaluateAsInitializer.

---

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

3 Files Affected:

• (modified) clang/lib/AST/ExprConstant.cpp (+9-7)
• (modified) clang/lib/AST/Interp/ByteCodeExprGen.h (+7-1)
• (modified) clang/test/AST/Interp/literals.cpp (+1-3)

diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 5947805f9576ff8..2a8b81c12555e94 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -15561,12 +15561,14 @@ bool Expr::EvaluateAsConstantExpr(EvalResult &Result, const ASTContext &Ctx,
   // this doesn't escape.
   MaterializeTemporaryExpr BaseMTE(T, const_cast<Expr*>(this), true);
   APValue::LValueBase Base(&BaseMTE);
-
   Info.setEvaluatingDecl(Base, Result.Val);
-  LValue LVal;
-  LVal.set(Base);
 
-  {
+  if (Info.EnableNewConstInterp) {
+    if (!Info.Ctx.getInterpContext().evaluateAsRValue(Info, this, Result.Val))
+      return false;
+  } else {
+    LValue LVal;
+    LVal.set(Base);
     // C++23 [intro.execution]/p5
     // A full-expression is [...] a constant-expression
     // So we need to make sure temporary objects are destroyed after having
@@ -15575,10 +15577,10 @@ bool Expr::EvaluateAsConstantExpr(EvalResult &Result, const ASTContext &Ctx,
     if (!::EvaluateInPlace(Result.Val, Info, LVal, this) ||
         Result.HasSideEffects || !Scope.destroy())
       return false;
-  }
 
-  if (!Info.discardCleanups())
-    llvm_unreachable("Unhandled cleanup; missing full expression marker?");
+    if (!Info.discardCleanups())
+      llvm_unreachable("Unhandled cleanup; missing full expression marker?");
+  }
 
   if (!CheckConstantExpression(Info, getExprLoc(), getStorageType(Ctx, this),
                                Result.Val, Kind))
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.h b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 83986d3dd579ed6..f40eb104f128da8 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -129,7 +129,13 @@ class ByteCodeExprGen : public ConstStmtVisitor<ByteCodeExprGen<Emitter>, bool>,
 
   /// Classifies a type.
   std::optional<PrimType> classify(const Expr *E) const {
-    return E->isGLValue() ? PT_Ptr : classify(E->getType());
+    if (E->isGLValue()) {
+      if (E->getType()->isFunctionType())
+        return PT_FnPtr;
+      return PT_Ptr;
+    }
+
+    return classify(E->getType());
   }
   std::optional<PrimType> classify(QualType Ty) const {
     return Ctx.classify(Ty);
diff --git a/clang/test/AST/Interp/literals.cpp b/clang/test/AST/Interp/literals.cpp
index ba24955d14503be..3ff4d4364c27314 100644
--- a/clang/test/AST/Interp/literals.cpp
+++ b/clang/test/AST/Interp/literals.cpp
@@ -261,15 +261,13 @@ namespace SizeOf {
 #if __cplusplus >= 202002L
   /// FIXME: The following code should be accepted.
   consteval int foo(int n) { // ref-error {{consteval function never produces a constant expression}}
-    return sizeof(int[n]); // ref-note 3{{not valid in a constant expression}} \
-                           // expected-note {{not valid in a constant expression}}
+    return sizeof(int[n]); // ref-note 3{{not valid in a constant expression}}
   }
   constinit int var = foo(5); // ref-error {{not a constant expression}} \
                               // ref-note 2{{in call to}} \
                               // ref-error {{does not have a constant initializer}} \
                               // ref-note {{required by 'constinit' specifier}} \
                               // expected-error  {{is not a constant expression}} \
-                              // expected-note {{in call to}} \
                               // expected-error {{does not have a constant initializer}} \
                               // expected-note {{required by 'constinit' specifier}} \
 

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[tbaederr]

Bot complains about lines I haven't touched 🤷

[erichkeane]

I don't have a good idea how a release note could be phrased here, but if you could try one, it would be appreciated and I think it is worth doing.

Side note: it would be really cool if at the end of the release cycle, you did a section in the release-notes with a 'status' of the new interpreter. Basically a 'things that don't work', or a 'percent of tests we have that work', etc?

[tbaederr]

I don't have a good idea how a release note could be phrased here, but if you could try one, it would be appreciated and I think it is worth doing.

You mean about this change specifically?

Side note: it would be really cool if at the end of the release cycle, you did a section in the release-notes with a 'status' of the new interpreter. Basically a 'things that don't work', or a 'percent of tests we have that work', etc?

I could force the new interpreter and run the test suite, but that would only be a very rough estimate.

[tbaederr]

Ping

[AaronBallman]

LGTM with unrelated changes pulled; I'm fine landing this PR after landing the fix for the function pointer classification changes if necessary.