Skip to content

[Clang] Instantiate constexpr function when they are needed.#173537

Open
cor3ntin wants to merge 10 commits intollvm:mainfrom
cor3ntin:corentin/sema-proxy
Open

[Clang] Instantiate constexpr function when they are needed.#173537
cor3ntin wants to merge 10 commits intollvm:mainfrom
cor3ntin:corentin/sema-proxy

Conversation

@cor3ntin
Copy link
Contributor

@cor3ntin cor3ntin commented Dec 25, 2025

This is a rework of #115168, with the sole aim of fixing #73232. The scaffolding is introduced here is necessary - but not sufficient for reflection.

Reading the comments on #115168, I think we need to separate multiple concerns:

  • Do we have access to Sema (which this PR provides)
  • Can we mutate the AST - which is something individual callers will have to figure out.

This does not address #59966, as the change is complicated enough that i want to explore it in a separate PR.

Fixes #73232
Fixes #35052
Fixes #100897

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:bytecode Issues for the clang bytecode constexpr interpreter labels Dec 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Dec 25, 2025

@llvm/pr-subscribers-clang

Author: Corentin Jabot (cor3ntin)

Changes

This is a rework of #115168, with the sole aim of fixing #73232. The scaffolding is introduced here is necessary - but not sufficient for reflection.

Reading the comments on #115168, I think we need to separate multiple concerns:

  • Do we have access to Sema (which this PR provides)
  • Can we mutate the AST - which is something individual callers will have to figure out.

This does not fix #59966, as the change is complicated enough that i want to explore it in a separate PR.

Fixes #73232


Patch is 20.99 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/173537.diff

14 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+6-5)
  • (modified) clang/include/clang/AST/ASTContext.h (+17)
  • (modified) clang/include/clang/Sema/Sema.h (+3)
  • (modified) clang/lib/AST/ASTContext.cpp (+7)
  • (modified) clang/lib/AST/ByteCode/Interp.cpp (+24-6)
  • (modified) clang/lib/AST/ByteCode/InterpState.cpp (+2)
  • (modified) clang/lib/AST/ByteCode/State.h (+2)
  • (modified) clang/lib/AST/ExprConstant.cpp (+38-4)
  • (modified) clang/lib/Interpreter/IncrementalParser.cpp (+5-1)
  • (modified) clang/lib/Parse/ParseAST.cpp (+6)
  • (modified) clang/lib/Sema/Sema.cpp (+30)
  • (modified) clang/test/SemaCXX/constexpr-late-instantiation.cpp (+79-5)
  • (modified) clang/test/SemaCXX/constexpr-subobj-initialization.cpp (+3-2)
  • (modified) clang/test/SemaCXX/cxx2b-consteval-propagate.cpp (+3-2)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 2319ff13f7864..2718dd61c139d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -331,8 +331,8 @@ Non-comprehensive list of changes in this release
   allocator-level heap organization strategies. A feature to instrument all
   allocation functions with a token ID can be enabled via the
   ``-fsanitize=alloc-token`` flag.
- 
-- A new generic byte swap builtin function ``__builtin_bswapg`` that extends the existing 
+
+- A new generic byte swap builtin function ``__builtin_bswapg`` that extends the existing
   __builtin_bswap{16,32,64} function family to support all standard integer types.
 
 - A builtin ``__builtin_infer_alloc_token(<args>, ...)`` is provided to allow
@@ -491,12 +491,12 @@ Improvements to Clang's diagnostics
   Objective-C method and block declarations when calling format functions. It is part
   of the format-nonliteral diagnostic (#GH60718)
 
-- Fixed a crash when enabling ``-fdiagnostics-format=sarif`` and the output 
+- Fixed a crash when enabling ``-fdiagnostics-format=sarif`` and the output
   carries messages like 'In file included from ...' or 'In module ...'.
   Now the include/import locations are written into `sarif.run.result.relatedLocations`.
 
-- Clang now generates a fix-it for C++20 designated initializers when the 
-  initializers do not match the declaration order in the structure. 
+- Clang now generates a fix-it for C++20 designated initializers when the
+  initializers do not match the declaration order in the structure.
 
 Improvements to Clang's time-trace
 ----------------------------------
@@ -592,6 +592,7 @@ Bug Fixes to C++ Support
 - Fixed a bug where our ``member-like constrained friend`` checking caused an incorrect analysis of lambda captures. (#GH156225)
 - Fixed a crash when implicit conversions from initialize list to arrays of
   unknown bound during constant evaluation. (#GH151716)
+- Instantiate constexpr functions as needed before they are evaluated. (#GH73232)
 - Support the dynamic_cast to final class optimization with pointer
   authentication enabled. (#GH152601)
 - Fix the check for narrowing int-to-float conversions, so that they are detected in
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 68205dd1c1fd9..206e482237342 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -44,6 +44,7 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/TinyPtrVector.h"
 #include "llvm/Support/TypeSize.h"
+#include <memory>
 #include <optional>
 
 namespace llvm {
@@ -215,6 +216,16 @@ struct TypeInfoChars {
   }
 };
 
+/// Interface that allows constant evaluator to call Sema
+/// and mutate the AST.
+struct SemaProxy {
+  virtual ~SemaProxy() = default;
+
+  virtual void
+  InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
+                                FunctionDecl *Function) = 0;
+};
+
 /// Holds long-lived AST nodes (such as types and decls) that can be
 /// referred to throughout the semantic analysis of a file.
 class ASTContext : public RefCountedBase<ASTContext> {
@@ -786,6 +797,8 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// Keeps track of the deallocated DeclListNodes for future reuse.
   DeclListNode *ListNodeFreeList = nullptr;
 
+  std::unique_ptr<SemaProxy> SemaProxyPtr;
+
 public:
   IdentifierTable &Idents;
   SelectorTable &Selectors;
@@ -1409,6 +1422,10 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// with this AST context, if any.
   ASTMutationListener *getASTMutationListener() const { return Listener; }
 
+  SemaProxy *getSemaProxy();
+
+  void setSemaProxy(std::unique_ptr<SemaProxy> Proxy);
+
   void PrintStats() const;
   const SmallVectorImpl<Type *>& getTypes() const { return Types; }
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c9ad6860dc625..bd1277a271906 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -909,6 +909,9 @@ class Sema final : public SemaBase {
   /// initialized but before it parses anything.
   void Initialize();
 
+  void RegisterSemaProxy();
+  void UnregisterSemaProxy();
+
   /// This virtual key function only exists to limit the emission of debug info
   /// describing the Sema class. GCC and Clang only emit debug info for a class
   /// with a vtable when the vtable is emitted. Sema is final and not
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index f52470a4d7458..a2768a9dc06b1 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -926,6 +926,13 @@ ASTContext::setExternalSource(IntrusiveRefCntPtr<ExternalASTSource> Source) {
   ExternalSource = std::move(Source);
 }
 
+SemaProxy *ASTContext::getSemaProxy() { return SemaProxyPtr.get(); }
+
+void ASTContext::setSemaProxy(std::unique_ptr<SemaProxy> Proxy) {
+  assert((!SemaProxyPtr || !Proxy) && "SemaProxy already set");
+  SemaProxyPtr = std::move(Proxy);
+}
+
 void ASTContext::PrintStats() const {
   llvm::errs() << "\n*** AST Context Stats:\n";
   llvm::errs() << "  " << Types.size() << " types total.\n";
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index 889ac1e1a9a7e..f8e10ec2c9d39 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "Interp.h"
+#include "ByteCode/Source.h"
 #include "Compiler.h"
 #include "Function.h"
 #include "InterpFrame.h"
@@ -1526,13 +1527,30 @@ bool CheckFunctionDecl(InterpState &S, CodePtr OpPC, const FunctionDecl *FD) {
   return diagnoseCallableDecl(S, OpPC, FD);
 }
 
-static void compileFunction(InterpState &S, const Function *Func) {
-  const FunctionDecl *Definition = Func->getDecl()->getDefinition();
-  if (!Definition)
+static void compileFunction(InterpState &S, const Function *Func,
+                            SourceLocation Loc) {
+
+  const FunctionDecl *Fn = Func->getDecl();
+
+  // [C++26] [temp.inst] p5
+  // [...] the function template specialization is implicitly instantiated
+  // when the specialization is referenced in a context that requires a function
+  // definition to exist or if the existence of the definition affects the
+  // semantics of the program.
+  if (!Fn->isDefined() && Fn->isImplicitlyInstantiable() && Fn->isConstexpr() &&
+      S.inConstantContext() && !S.TryConstantInitialization &&
+      !S.checkingPotentialConstantExpression()) {
+    SemaProxy *SP = S.getASTContext().getSemaProxy();
+    if (!SP)
+      return;
+    SP->InstantiateFunctionDefinition(Loc, const_cast<FunctionDecl *>(Fn));
+  }
+  Fn = Fn->getDefinition();
+  if (!Fn)
     return;
 
   Compiler<ByteCodeEmitter>(S.getContext(), S.P)
-      .compileFunc(Definition, const_cast<Function *>(Func));
+      .compileFunc(Fn, const_cast<Function *>(Func));
 }
 
 bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
@@ -1558,7 +1576,7 @@ bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
   }
 
   if (!Func->isFullyCompiled())
-    compileFunction(S, Func);
+    compileFunction(S, Func, S.Current->getLocation(OpPC));
 
   if (!CheckCallable(S, OpPC, Func))
     return false;
@@ -1630,7 +1648,7 @@ bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
   }
 
   if (!Func->isFullyCompiled())
-    compileFunction(S, Func);
+    compileFunction(S, Func, S.Current->getLocation(OpPC));
 
   if (!CheckCallable(S, OpPC, Func))
     return cleanup();
diff --git a/clang/lib/AST/ByteCode/InterpState.cpp b/clang/lib/AST/ByteCode/InterpState.cpp
index a95916cd63981..ff201cb6151b1 100644
--- a/clang/lib/AST/ByteCode/InterpState.cpp
+++ b/clang/lib/AST/ByteCode/InterpState.cpp
@@ -22,6 +22,7 @@ InterpState::InterpState(State &Parent, Program &P, InterpStack &Stk,
     : Parent(Parent), M(M), P(P), Stk(Stk), Ctx(Ctx), BottomFrame(*this),
       Current(&BottomFrame) {
   InConstantContext = Parent.InConstantContext;
+  TryConstantInitialization = Parent.TryConstantInitialization;
   CheckingPotentialConstantExpression =
       Parent.CheckingPotentialConstantExpression;
   CheckingForUndefinedBehavior = Parent.CheckingForUndefinedBehavior;
@@ -34,6 +35,7 @@ InterpState::InterpState(State &Parent, Program &P, InterpStack &Stk,
       BottomFrame(*this, Func, nullptr, CodePtr(), Func->getArgSize()),
       Current(&BottomFrame) {
   InConstantContext = Parent.InConstantContext;
+  TryConstantInitialization = Parent.TryConstantInitialization;
   CheckingPotentialConstantExpression =
       Parent.CheckingPotentialConstantExpression;
   CheckingForUndefinedBehavior = Parent.CheckingForUndefinedBehavior;
diff --git a/clang/lib/AST/ByteCode/State.h b/clang/lib/AST/ByteCode/State.h
index 0695c61c07a05..376d120c3a9d9 100644
--- a/clang/lib/AST/ByteCode/State.h
+++ b/clang/lib/AST/ByteCode/State.h
@@ -168,6 +168,8 @@ class State {
   /// is set; this is used when evaluating ICEs in C.
   bool CheckingForUndefinedBehavior = false;
 
+  bool TryConstantInitialization = false;
+
   EvaluationMode EvalMode;
 
 private:
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6ccd57bdc4df8..d9c1c8d6d6b8a 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -43,6 +43,7 @@
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/CurrentSourceLocExprScope.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/InferAlloc.h"
 #include "clang/AST/OSLog.h"
@@ -53,6 +54,7 @@
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "llvm/ADT/APFixedPoint.h"
@@ -7064,6 +7066,28 @@ static bool handleTrivialCopy(EvalInfo &Info, const ParmVarDecl *Param,
       CopyObjectRepresentation);
 }
 
+static void InstantiateFunctionBeforeCall(const FunctionDecl *FD,
+                                          EvalInfo &Info, SourceLocation Loc) {
+
+  // [C++26] [temp.inst] p5
+  // [...] the function template specialization is implicitly instantiated
+  // when the specialization is referenced in a context that requires a function
+  // definition to exist or if the existence of the definition affects the
+  // semantics of the program.
+
+  if (!FD->isDefined() && FD->isImplicitlyInstantiable() && FD->isConstexpr() &&
+      Info.InConstantContext && !Info.TryConstantInitialization &&
+      !Info.checkingPotentialConstantExpression()) {
+
+    SemaProxy *SP = Info.getASTContext().getSemaProxy();
+    // Try to instantiate the definition if Sema is available
+    // (i.e during the initial parse of the TU).
+    if (SP) {
+      SP->InstantiateFunctionDefinition(Loc, const_cast<FunctionDecl *>(FD));
+    }
+  }
+}
+
 /// Evaluate a function call.
 static bool HandleFunctionCall(SourceLocation CallLoc,
                                const FunctionDecl *Callee,
@@ -7457,6 +7481,8 @@ static bool HandleDestructionImpl(EvalInfo &Info, SourceRange CallRange,
   if (!Info.CheckCallLimit(CallRange.getBegin()))
     return false;
 
+  InstantiateFunctionBeforeCall(DD, Info, CallRange.getBegin());
+
   const FunctionDecl *Definition = nullptr;
   const Stmt *Body = DD->getBody(Definition);
 
@@ -8922,10 +8948,13 @@ class ExprEvaluatorBase
              CallScope.destroy();
     }
 
-    const FunctionDecl *Definition = nullptr;
-    Stmt *Body = FD->getBody(Definition);
     SourceLocation Loc = E->getExprLoc();
 
+    InstantiateFunctionBeforeCall(FD, Info, Loc);
+
+    const FunctionDecl *Definition = nullptr;
+    const Stmt *Body = FD->getBody(Definition);
+
     // Treat the object argument as `this` when evaluating defaulted
     // special menmber functions
     if (FD->hasCXXExplicitFunctionObjectParameter())
@@ -11443,8 +11472,10 @@ bool RecordExprEvaluator::VisitCXXConstructExpr(const CXXConstructExpr *E,
     return handleDefaultInitValue(T, Result);
   }
 
+  InstantiateFunctionBeforeCall(FD, Info, E->getBeginLoc());
+
   const FunctionDecl *Definition = nullptr;
-  auto Body = FD->getBody(Definition);
+  const Stmt *Body = FD->getBody(Definition);
 
   if (!CheckConstexprFunction(Info, E->getExprLoc(), FD, Definition, Body))
     return false;
@@ -11484,8 +11515,9 @@ bool RecordExprEvaluator::VisitCXXInheritedCtorInitExpr(
   if (FD->isInvalidDecl() || FD->getParent()->isInvalidDecl())
     return false;
 
+  InstantiateFunctionBeforeCall(FD, Info, E->getBeginLoc());
   const FunctionDecl *Definition = nullptr;
-  auto Body = FD->getBody(Definition);
+  const Stmt *Body = FD->getBody(Definition);
 
   if (!CheckConstexprFunction(Info, E->getExprLoc(), FD, Definition, Body))
     return false;
@@ -20707,6 +20739,8 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
                     : EvaluationMode::ConstantFold);
   Info.setEvaluatingDecl(VD, Value);
   Info.InConstantContext = IsConstantInitialization;
+  Info.TryConstantInitialization =
+      !VD->isConstexpr() && !VD->hasAttr<ConstInitAttr>();
 
   SourceLocation DeclLoc = VD->getLocation();
   QualType DeclTy = VD->getType();
diff --git a/clang/lib/Interpreter/IncrementalParser.cpp b/clang/lib/Interpreter/IncrementalParser.cpp
index bf08911e23533..5a24342e73ca4 100644
--- a/clang/lib/Interpreter/IncrementalParser.cpp
+++ b/clang/lib/Interpreter/IncrementalParser.cpp
@@ -42,9 +42,13 @@ IncrementalParser::IncrementalParser(CompilerInstance &Instance,
     External->StartTranslationUnit(Consumer);
 
   P->Initialize();
+  S.RegisterSemaProxy();
 }
 
-IncrementalParser::~IncrementalParser() { P.reset(); }
+IncrementalParser::~IncrementalParser() {
+  S.UnregisterSemaProxy();
+  P.reset();
+}
 
 llvm::Expected<TranslationUnitDecl *>
 IncrementalParser::ParseOrWrapTopLevelDecl() {
diff --git a/clang/lib/Parse/ParseAST.cpp b/clang/lib/Parse/ParseAST.cpp
index c8ee625eb57ad..1cea282f13a00 100644
--- a/clang/lib/Parse/ParseAST.cpp
+++ b/clang/lib/Parse/ParseAST.cpp
@@ -21,6 +21,7 @@
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaConsumer.h"
 #include "clang/Sema/TemplateInstCallback.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/TimeProfiler.h"
 #include <cstdio>
@@ -161,6 +162,11 @@ void clang::ParseAST(Sema &S, bool PrintStats, bool SkipFunctionBodies) {
       return M;
     });
     P.Initialize();
+
+    auto Unregister =
+        llvm::make_scope_exit([&S]() { S.UnregisterSemaProxy(); });
+    S.RegisterSemaProxy();
+
     Parser::DeclGroupPtrTy ADecl;
     Sema::ModuleImportState ImportState;
     EnterExpressionEvaluationContext PotentiallyEvaluated(
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 6eea8f6e9d97e..e148c23f15516 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -75,11 +75,14 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Support/TimeProfiler.h"
+#include <memory>
 #include <optional>
 
 using namespace clang;
 using namespace sema;
 
+static std::unique_ptr<SemaProxy> getSemaProxyImplementation(Sema &SemaRef);
+
 SourceLocation Sema::getLocForEndOfToken(SourceLocation Loc, unsigned Offset) {
   return Lexer::getLocForEndOfToken(Loc, Offset, SourceMgr, LangOpts);
 }
@@ -622,6 +625,15 @@ Sema::~Sema() {
   SemaPPCallbackHandler->reset();
 }
 
+void Sema::RegisterSemaProxy() {
+  // Let the AST context relies on Sema for
+  // ast mutations features that require semantic analysis
+  // (lazy instantiation, reflection, etc).
+  Context.setSemaProxy(getSemaProxyImplementation(*this));
+}
+
+void Sema::UnregisterSemaProxy() { Context.setSemaProxy({}); }
+
 void Sema::runWithSufficientStackSpace(SourceLocation Loc,
                                        llvm::function_ref<void()> Fn) {
   StackHandler.runWithSufficientStackSpace(Loc, Fn);
@@ -2958,3 +2970,21 @@ Attr *Sema::CreateAnnotationAttr(const ParsedAttr &AL) {
 
   return CreateAnnotationAttr(AL, Str, Args);
 }
+
+class SemaProxyImplementation final : public SemaProxy {
+private:
+  Sema &SemaRef;
+
+public:
+  SemaProxyImplementation(Sema &SemaRef) : SemaRef(SemaRef) {}
+  void InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
+                                     FunctionDecl *Function) override {
+    SemaRef.InstantiateFunctionDefinition(
+        PointOfInstantiation, Function, /*Recursive=*/true,
+        /*DefinitionRequired=*/true, /*AtEndOfTU=*/false);
+  }
+};
+
+std::unique_ptr<SemaProxy> getSemaProxyImplementation(Sema &SemaRef) {
+  return std::make_unique<SemaProxyImplementation>(SemaRef);
+}
diff --git a/clang/test/SemaCXX/constexpr-late-instantiation.cpp b/clang/test/SemaCXX/constexpr-late-instantiation.cpp
index 9aec0c90e61dc..59cfbfdff0e03 100644
--- a/clang/test/SemaCXX/constexpr-late-instantiation.cpp
+++ b/clang/test/SemaCXX/constexpr-late-instantiation.cpp
@@ -1,16 +1,90 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify
-// RUN: %clang_cc1 %s -fexperimental-new-constant-interpreter -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=c++14 -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=c++2c -fsyntax-only -verify
+
+// RUN: %clang_cc1 %s -std=c++14 -fsyntax-only -fexperimental-new-constant-interpreter -verify
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -fexperimental-new-constant-interpreter -verify
+// RUN: %clang_cc1 %s -std=c++2c -fsyntax-only -fexperimental-new-constant-interpreter -verify
 
 template <typename T>
-constexpr T foo(T a);   // expected-note {{declared here}}
+constexpr T foo(T a);   // expected-note {{explicit instantiation refers here}}
 
 int main() {
   int k = foo<int>(5);  // Ok
-  constexpr int j =     // expected-error {{constexpr variable 'j' must be initialized by a constant expression}}
-          foo<int>(5);  // expected-note {{undefined function 'foo<int>' cannot be used in a constant expression}}
+  constexpr int j =
+          foo<int>(5);  // expected-error {{explicit instantiation of undefined function template 'foo'}} \
+                        // expected-error {{constexpr variable 'j' must be initialized by a constant expression}}
 }
 
 template <typename T>
 constexpr T foo(T a) {
   return a;
 }
+
+
+namespace GH73232 {
+namespace ex1 {
+template <typename T>
+constexpr void g(T);
+
+constexpr int f() {
+  g(0);
+  return 0;
+}
+
+template <typename T>
+constexpr void g(T) {}
+
+constexpr auto z = f();
+}
+
+namespace ex2 {
+template <typename> constexpr static void fromType();
+
+void registerConverter() { fromType<int>(); }
+template <typename> struct QMetaTypeId  {};
+template <typename T> constexpr void fromType() {
+  (void)QMetaTypeId<T>{};
+}
+template <> struct QMetaTypeId<int> {};
+} // namespace ex2
+
+namespace ex3 {
+
+#if __cplusplus > 202302L
+struct A {
+    consteval A(int i) {
+        chk(i);
+    }
+    constexpr void chk(auto) {}
+};
+A a{1};
+
+#endif
+
+}
+
+} // namespace GH73232
+
+
+namespace GH156255 {
+
+class X
+{
+public:
+    constexpr int f( int x ) const
+    {
+        return g( x );
+    }
+
+private:
+
+    template<class T>
+    constexpr T g( T x ) const
+    {
+        return x;
+    }
+};
+
+constexpr int x = X().f( 1 );
+}
diff --git a/clang/test/SemaCXX/constexpr-subobj-initialization.cpp b/clang/test/SemaCXX/constexpr-subobj-initialization.cpp
index f0252df1e2ce1..c2e51d3920f6a 100644
--- a/clang/test/SemaCXX/constexpr-subobj-initialization.cpp
+++ b/clang/test/SemaCXX/constexpr-subobj-initialization.cpp
@@ -47,12 +47,13 @@ constexpr Bar bb; // expected-error {{must be initialized by a constant expressi
 
 template <typename Ty>
 struct Baz {
-  constexpr Baz(); // expected-note {{declared here}}
+  constexpr Baz(); // expected-note {{explicit instantiation refers here}}
 };
 
 struct Quux : Baz<Foo>, private Bar {
   int i;
-  constexpr Quux() : i(12) {} // expected-note {{undefined constructor 'Baz' cannot be used in a constant expression}}
+  constexpr Quux() : i(12) {} // expected-error {{explicit instantiation of undefined member function 'Baz' of class template 'Baz<Foo>'}} \
+                              // expected-note {{subexpression not valid in a constant expression}}
 };
 
 constexpr Quux qx; // expect...
[truncated]

Info.setEvaluatingDecl(VD, Value);
Info.InConstantContext = IsConstantInitialization;
Info.TryConstantInitialization =
!VD->isConstexpr() && !VD->hasAttr<ConstInitAttr>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this mean we only do it for initializers? What if I want to call such a function in a static_assert call? Shouldn't that work too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's the other way around - when trying to initialize a constant-initialized non-constexpr variable we do not want to force an instantiation (it may not yet be available or a more specialized definition may be provided later)

Comment on lines +1540 to +1542
if (!Fn->isDefined() && Fn->isImplicitlyInstantiable() && Fn->isConstexpr() &&
S.inConstantContext() && !S.TryConstantInitialization &&
!S.checkingPotentialConstantExpression()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this entire condition into a helper function somewhere (maybe make it a member of State)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I'm not sure I can find a nice spot for it - we do it once per constant evaluation engine.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I commented above, I think there is value in this being TWO functions for htis condition.

ONE for hte function condition and one for the State. I'm not clever enough/nor understand it well enough to come up with good names though. BUT something where this is:

if (Fn->canBeAndNeedsConstexprEvaluationTimeInstantationOrSomethingOfThatSort() && S.evaluatingConstantExprSituationWhereWeProbablyShouldInstantiateConstexprFunctionsSometimesWhoKnowsWhatever())

might be nice.

Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are #35052 and related issues. Are they fixed now?

// (i.e during the initial parse of the TU).
if (SP) {
SP->InstantiateFunctionDefinition(Loc, const_cast<FunctionDecl *>(FD));
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should have a warning for the else branch, so that users know the behavior might be wrong (generally a missed instantiation should be our bug)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else case is tooling (ie calling evaluate from tidy or lldb) - I don;t know if a warning make sense in that case

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so? For status-quo, missed instantiation would always result in failures, and having extra warnings does give toolings a chance to inspect the issue.

return CreateAnnotationAttr(AL, Str, Args);
}

class SemaProxyImplementation final : public SemaProxy {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about SemaProxyForConstantEvaluator?

@cor3ntin
Copy link
Contributor Author

cor3ntin commented Jan 2, 2026

There are #35052 and related issues. Are they fixed now?

Thanks for letting me know about these. They are fixed, I added tests

@github-actions
Copy link

github-actions bot commented Jan 2, 2026

🪟 Windows x64 Test Results

  • 54490 tests passed
  • 2319 tests skipped

✅ The build succeeded and all tests passed.

@github-actions
Copy link

github-actions bot commented Jan 2, 2026

🐧 Linux x64 Test Results

  • 114415 tests passed
  • 4581 tests skipped

✅ The build succeeded and all tests passed.

@cor3ntin cor3ntin force-pushed the corentin/sema-proxy branch 2 times, most recently from 67c3a9a to d136f8a Compare January 9, 2026 09:55
Copy link
Collaborator

@erichkeane erichkeane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reaosnable to me? Or at least as we can be for what sort of stuff this needs to do.

Though I DO wonder if the property:
!FD->isDefined() && FD->isImplicitlyInstantiable() && FD->isConstexpr()

should be a function inside of FunctionDecl with its own descriptive name. Those conditions there are pretty horrific to read.

CopyObjectRepresentation);
}

static void InstantiateFunctionBeforeCall(const FunctionDecl *FD,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is perhaps a try that should be in the name of some sort.

@cor3ntin cor3ntin force-pushed the corentin/sema-proxy branch from d136f8a to ea6211a Compare January 9, 2026 15:42
@cor3ntin
Copy link
Contributor Author

cor3ntin commented Jan 9, 2026

@erichkeane @Sirraide I realized we have a ExprConstShared.h so I put a function with the !FD->isDefined() && FD->isImplicitlyInstantiable() && FD->isConstexpr() in there

@Sirraide
Copy link
Member

Sirraide commented Jan 9, 2026

I realized we have a ExprConstShared.h so I put a function with the !FD->isDefined() && FD->isImplicitlyInstantiable() && FD->isConstexpr() in there

Oh I didn’t realise that that was a thing either; stgm.

@AaronBallman AaronBallman requested a review from Endilll January 9, 2026 17:08
This is a rework of llvm#115168, with the sole aim of fixing llvm#73232.
The scaffolding is introduced here is necessary - but not sufficient
for reflection.

Reading the comments on llvm#115168, I think we need to separate multiple
concerns:
   - Do we have access to Sema (which this PR provides)
   - Can we mutate the AST - which is something individual callers
     will have to figure out.

This does **not** fix llvm#59966, as the change is complicated enough
that i want to explore it in a separate PR.

Fixes llvm#73232
@cor3ntin cor3ntin force-pushed the corentin/sema-proxy branch from ea6211a to 95e3c7d Compare February 16, 2026 18:55
@cor3ntin cor3ntin force-pushed the corentin/sema-proxy branch from 95e3c7d to 8daac7b Compare February 16, 2026 19:14
@cor3ntin
Copy link
Contributor Author

I had chats with both @hubert-reinterpretcast and @katzdm

We concluded that this is consistent with proposed resolutions to CWG2166 and CWG2497 (for those who have access https://lists.isocpp.org/core/2025/12/19097.php).
Note that an actual wording for both of these issues have yet to be accepted.

So while at a previous clang WG (late 2025?) there was hope for a solution that would preserve the immutability of constant evaluation, I think we converged toward an acceptance (with more or less reluctancy) than this shipped had sailed and therefore the solution presented here was both acceptable and forward-consistent with future tweaks for reflection.

/// Keeps track of the deallocated DeclListNodes for future reuse.
DeclListNode *ListNodeFreeList = nullptr;

std::unique_ptr<SemaProxy> SemaProxyPtr;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you going for (basically) a global variable that holds SemaProxy, instead of passing it explicitly to Eval*() functions? Explicit passing is what discussions around #115168 settled at, and it would also better insulate us against issues like this one I recently filed against GCC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the description of the PR.
For reflection, we will need to pass around (or put on the stack or something) some sort of additional context (and/or modify EvaluationContext, which would be better).

It's important to separate "do we have access to sema" from "can we do that specific thing" - notably because sema can do different things some but not all of which can be valid in a particular context - here we need to check for example that we are in a constant context, not doing trial evaluation, not checking for overflow, etc

template <typename T>
constexpr void g(T) {}

constexpr auto z = f();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the working code, I think comments narrating the behavior and why it should be working would be very helpful for all these additional tests. So we can see specific behavior changes and we can see any implementation decisions documents explicitly in the tests. This will help us later on when we go back and try and build any rationale back for changes.

Right now, I am just walking through the tests but I am not sure what lines are key and whether I should be surprised or not.

Comment on lines +1425 to +1428
SemaProxy *getSemaProxy();

void setSemaProxy(std::unique_ptr<SemaProxy> Proxy);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to document these two functions (like what getASTMutationListener has), as ASTContext is exposed to (out-of-tree) users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

Comment on lines +929 to +934
SemaProxy *ASTContext::getSemaProxy() { return SemaProxyPtr.get(); }

void ASTContext::setSemaProxy(std::unique_ptr<SemaProxy> Proxy) {
assert((!SemaProxyPtr || !Proxy) && "SemaProxy already set");
SemaProxyPtr = std::move(Proxy);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we inline the getter/setter?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setSemaProxy is only called once, but i inlined getSemaProxy, that might be beneficial indeed!

@github-actions
Copy link

github-actions bot commented Mar 2, 2026

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

@jyknight
Copy link
Member

jyknight commented Mar 5, 2026

This PR does resolve some of the errors in this area, but does not seem to resolve my previously-reported issue:

#include <memory>

struct X;

struct Y {
    Y() {}
    std::unique_ptr<X> member_;
};

struct X {  int a; };

With this patch applied, clang -std=c++23 -fsyntax-only still emits the following diagnostics:

include/c++/v1/__memory/unique_ptr.h:72:19: error: 
      invalid application of 'sizeof' to an incomplete type 'X'
   72 |     static_assert(sizeof(_Tp) >= 0, "cannot delete an incomplete type");
      |                   ^~~~~~~~~~~
include/c++/v1/__memory/unique_ptr.h:288:7: note: 
      in instantiation of member function 'std::default_delete<X>::operator()' requested here
  288 |       __deleter_(__tmp);
      |       ^
include/c++/v1/__memory/unique_ptr.h:254:71: note: 
      in instantiation of member function 'std::unique_ptr<X>::reset' requested here
  254 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
      |                                                                       ^
test.cc:6:5: note: in instantiation of member function 'std::unique_ptr<X>::~unique_ptr' requested here
    6 |     Y() {}
      |     ^
test.cc:3:8: note: forward declaration of 'X'
    3 | struct X;
      |        ^
1 error generated.

gcc -std=c++23 compiles it successfully, as does clang -std=c++20 (because ~unique_ptr isn't marked constexpr in C++20).

Here's a test-case without library headers:

template <typename T>
struct unique_ptr {
    constexpr ~unique_ptr() {
        static_assert(sizeof(T));
    }
    T* ptr;
};

struct X;

struct Y {
    Y() {}
    unique_ptr<X> member_;
};

struct X {  int a; };

@katzdm
Copy link
Contributor

katzdm commented Mar 5, 2026

Here's a test-case without library headers:

template <typename T>
struct unique_ptr {
    constexpr ~unique_ptr() {
        static_assert(sizeof(T));
    }
    T* ptr;
};

struct X;

struct Y {
    Y() {}
    unique_ptr<X> member_;
};

struct X {  int a; };

My reading of this is: unique_ptr<X>::~unique_ptr() is used by Y::~Y(); it has two points of instantiation:

  1. immediately after the definition of Y and
  2. at the end of the translation unit.

An implementation is free to choose either, so a program (like the one above) for which the choice matters is IFNDR. Clang is choosing (1) when the function is constexpr (see here), whereas GCC chooses (2). In that sense, there is no problem here.

But if we wish to pursue greater compatibility with GCC, then Clang is, of course, also free to choose (2) - it's worth noting the documented rationale (linked above) for why (1) has up until this time been preferred:

else if (Func->isConstexpr())
  // Do not defer instantiations of constexpr functions, to avoid the
  // expression evaluator needing to call back into Sema if it sees a
  // call to such a function.
  InstantiateFunctionDefinition(PointOfInstantiation, Func);

@cor3ntin - Now that the expression evaluator can call back into Sema, is it worth revisiting this design? Perhaps (in a separate follow-on PR?) you might try removing the above early-instantiation. What do you think?

@cor3ntin
Copy link
Contributor Author

cor3ntin commented Mar 5, 2026

The plan is to do a follow up once this lands ( as mentioned in the description)

Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM plus some nits.

Comment on lines +225 to +227
virtual void
instantiateFunctionDefinition(SourceLocation PointOfInstantiation,
FunctionDecl *Function) = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's less necessary to make it an interface?

Users are not likely going to have their own instantiation logic. And the no-op behavior is already defaulted with null SemaProxy. WDYT?

/// with this AST context, if any.
ASTMutationListener *getASTMutationListener() const { return Listener; }

/// Returns a SemaProxy*, i.e an object through which interact with Sema.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Returns a SemaProxy*, i.e an object through which interact with Sema.
/// Returns a SemaProxy*, i.e. an object through which to interact with Sema.

Comment on lines +1443 to +1445
/// This will return null, unless setSemaProxy has been called.
/// As Sema is only available during parsing, getSemaProxy will return null
/// during CodeGen and when using the AST from tooling.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// This will return null, unless setSemaProxy has been called.
/// As Sema is only available during parsing, getSemaProxy will return null
/// during CodeGen and when using the AST from tooling.
/// This will return a SemaProxy if available, e.g. during parsing.
/// This is currently unavailable during CodeGen or when using the AST from tooling.

/// during CodeGen and when using the AST from tooling.
SemaProxy *getSemaProxy() { return SemaProxyPtr.get(); }

/// Set the SemaProxy instance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Set the SemaProxy instance
/// Set the SemaProxy instance.

@jyknight
Copy link
Member

jyknight commented Mar 6, 2026

The plan is to do a follow up once this lands ( as mentioned in the description)

Ah, sorry! I saw that, but I didn't understand that my example was an instance of the not-expected-to-be-fixed issue, rather than an example of the expected-to-be-fixed issue.

At least for me, it was unclear what the distinguishing factors are between "fixed in this PR" vs "deferred to followup PR" were.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:bytecode Issues for the clang bytecode constexpr interpreter clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

None yet