https://github.com/cor3ntin updated https://github.com/llvm/llvm-project/pull/73463
>From a1d36de81074ad17bc5d22a4c5906f5a0bfa65c4 Mon Sep 17 00:00:00 2001 From: Corentin Jabot <corentinja...@gmail.com> Date: Sun, 26 Nov 2023 22:47:51 +0100 Subject: [PATCH 1/2] [Clang] Eagerly instantiate used constexpr function upon definition. Despite CWG2497 not being resolved, it is reasonable to expect the following code to compile (and which is supported by other compilers) ```cpp template<typename T> constexpr T f(); constexpr int g() { return f<int>(); } // #1 template<typename T> constexpr T f() { return 123; } int k[g()]; // #2 ``` To that end, we eagerly instantiate all referenced specializations of constexpr functions when they are defined. We maintain a map of (pattern, [instantiations]) independant of `PendingInstantiations` to avoid having to iterate that list after each function definition. We should apply the same logic to constexpr variables, but I wanted to keep the PR small. Fixes #73232 --- clang/docs/ReleaseNotes.rst | 5 ++++ clang/include/clang/Sema/Sema.h | 11 +++++++ clang/lib/Sema/SemaDecl.cpp | 3 ++ clang/lib/Sema/SemaExpr.cpp | 9 ++++-- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 17 +++++++++++ .../SemaCXX/cxx2b-consteval-propagate.cpp | 8 +++-- .../instantiate-used-constexpr-function.cpp | 30 +++++++++++++++++++ 7 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 clang/test/SemaTemplate/instantiate-used-constexpr-function.cpp diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 7c909ac3cab6419..f46891364e4764b 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -783,6 +783,11 @@ Bug Fixes to C++ Support completes (except deduction guides). Fixes: (`#59827 <https://github.com/llvm/llvm-project/issues/59827>`_) +- Clang now immediately instantiates function template specializations + at the end of the definition of the corresponding function template + when the definition appears after the first point of instantiation. + (`#73232 <https://github.com/llvm/llvm-project/issues/73232>`_) + Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ - Fixed an import failure of recursive friend class template. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index f7c9d0e2e6412b7..091e1e3b4c1fd64 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -59,6 +59,7 @@ #include "clang/Sema/TypoCorrection.h" #include "clang/Sema/Weak.h" #include "llvm/ADT/ArrayRef.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SetVector.h" #include "llvm/ADT/SmallBitVector.h" #include "llvm/ADT/SmallPtrSet.h" @@ -10078,6 +10079,12 @@ class Sema final { /// but have not yet been performed. std::deque<PendingImplicitInstantiation> PendingInstantiations; + /// Track constexpr functions referenced before they are (lexically) defined. + /// The key is the pattern, associated with a list of specialisations that + /// need to be instantiated when the pattern is defined. + llvm::DenseMap<NamedDecl *, SmallVector<NamedDecl *>> + PendingInstantiationsOfConstexprEntities; + /// Queue of implicit template instantiations that cannot be performed /// eagerly. SmallVector<PendingImplicitInstantiation, 1> LateParsedInstantiations; @@ -10396,6 +10403,10 @@ class Sema final { bool Recursive = false, bool DefinitionRequired = false, bool AtEndOfTU = false); + + void InstantiateFunctionTemplateSpecializations( + SourceLocation PointOfInstantiation, FunctionDecl *Template); + VarTemplateSpecializationDecl *BuildVarTemplateInstantiation( VarTemplateDecl *VarTemplate, VarDecl *FromVar, const TemplateArgumentList &TemplateArgList, diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 23dd8ae15c16583..1030ba0d21b1906 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -16255,6 +16255,9 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt *Body, if (FD && !FD->isDeleted()) checkTypeSupport(FD->getType(), FD->getLocation(), FD); + if (FD && FD->isConstexpr() && FD->isTemplated()) + InstantiateFunctionTemplateSpecializations(FD->getEndLoc(), FD); + return dcl; } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index fc39d6149c1cc65..37b0d0eed35845c 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -19047,12 +19047,17 @@ void Sema::MarkFunctionReferenced(SourceLocation Loc, FunctionDecl *Func, CodeSynthesisContexts.size()) PendingLocalImplicitInstantiations.push_back( std::make_pair(Func, PointOfInstantiation)); - else if (Func->isConstexpr()) + 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); - else { + if (!Func->isDefined()) { + PendingInstantiationsOfConstexprEntities + [Func->getTemplateInstantiationPattern()->getCanonicalDecl()] + .push_back(Func); + } + } else { Func->setInstantiationIsPending(true); PendingInstantiations.push_back( std::make_pair(Func, PointOfInstantiation)); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 08f4ba00fc9f7de..9afbf139a1c2b88 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -6481,6 +6481,23 @@ void Sema::PerformPendingInstantiations(bool LocalOnly) { PendingInstantiations.swap(delayedPCHInstantiations); } +// Instantiate all referenced specializations of the given function template +// definition. This make sure that function template that are defined after the +// point of instantiation of their used can be evaluated after they are defined. +// see CWG2497. +void Sema::InstantiateFunctionTemplateSpecializations( + SourceLocation PointOfInstantiation, FunctionDecl *Tpl) { + auto It = + PendingInstantiationsOfConstexprEntities.find(Tpl->getCanonicalDecl()); + if (It == PendingInstantiationsOfConstexprEntities.end()) + return; + for (NamedDecl *Fun : It->second) { + InstantiateFunctionDefinition(PointOfInstantiation, + cast<FunctionDecl>(Fun)); + } + PendingInstantiationsOfConstexprEntities.erase(It); +} + void Sema::PerformDependentDiagnostics(const DeclContext *Pattern, const MultiLevelTemplateArgumentList &TemplateArgs) { for (auto *DD : Pattern->ddiags()) { diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp index 531a62622873357..a2b4039a31db778 100644 --- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp +++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp @@ -105,13 +105,15 @@ template <typename T> constexpr int f(T t); auto a = &f<char>; -auto b = &f<int>; // expected-error {{immediate function 'f<int>' used before it is defined}} \ - // expected-note {{in instantiation of function template specialization}} +auto b = &f<int>; // expected-error {{immediate function 'f<int>' used before it is defined}} template <typename T> constexpr int f(T t) { // expected-note {{'f<int>' defined here}} return id(t); // expected-note {{'f<int>' is an immediate function because its body contains a call to a consteval function 'id' and that call is not a constant expression}} -} +} // expected-note {{in instantiation of function template specialization}} + + + } namespace constructors { diff --git a/clang/test/SemaTemplate/instantiate-used-constexpr-function.cpp b/clang/test/SemaTemplate/instantiate-used-constexpr-function.cpp new file mode 100644 index 000000000000000..61a7fb01376805f --- /dev/null +++ b/clang/test/SemaTemplate/instantiate-used-constexpr-function.cpp @@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// expected-no-diagnostics + +namespace GH73232 { + +template <typename _CharT> +struct basic_string { + constexpr void _M_construct(); + constexpr basic_string() { + _M_construct(); + } +}; + +basic_string<char> a; + +template <typename _CharT> +constexpr void basic_string<_CharT>::_M_construct(){} +constexpr basic_string<char> str{}; + +template <typename T> +constexpr void g(T); + +constexpr int f() { g(0); return 0; } + +template <typename T> +constexpr void g(T) {} + +constexpr int z = f(); + +} >From b0724384cb9ccb3ac47cf5988a97acfc242ff1a6 Mon Sep 17 00:00:00 2001 From: Corentin Jabot <corentinja...@gmail.com> Date: Tue, 28 Nov 2023 19:47:46 +0100 Subject: [PATCH 2/2] WIP: Serialize pending constexr instantiations I wanted to avoid loading eagerly all function templates and their instantiations - even if ultimately `PendingInstantiations` - which contains the same information is going to be loaded at the end of the TU. So instead, when we find the definition of a constexpr function template we load the associated instantiation from the external modules at that point. I'm not sure there is a better way to get to a DeclID from a Decl* than to iterate over all redeclarations? I still need to cleanup but I'd like feedback on direction first! --- clang/include/clang/Sema/ExternalSemaSource.h | 3 +++ .../clang/Sema/MultiplexExternalSemaSource.h | 3 +++ .../include/clang/Serialization/ASTBitCodes.h | 4 +++ clang/include/clang/Serialization/ASTReader.h | 6 +++++ .../lib/Sema/MultiplexExternalSemaSource.cpp | 6 +++++ .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 22 +++++++++++----- clang/lib/Serialization/ASTReader.cpp | 25 +++++++++++++++++++ clang/lib/Serialization/ASTWriter.cpp | 16 ++++++++++++ .../instantiate-used-constexpr-function.cpp | 17 +++++++++++++ 9 files changed, 96 insertions(+), 6 deletions(-) create mode 100644 clang/test/PCH/instantiate-used-constexpr-function.cpp diff --git a/clang/include/clang/Sema/ExternalSemaSource.h b/clang/include/clang/Sema/ExternalSemaSource.h index 22d1ee2df115a6e..67bac7f0a5a91f5 100644 --- a/clang/include/clang/Sema/ExternalSemaSource.h +++ b/clang/include/clang/Sema/ExternalSemaSource.h @@ -181,6 +181,9 @@ class ExternalSemaSource : public ExternalASTSource { SmallVectorImpl<std::pair<ValueDecl *, SourceLocation> > &Pending) {} + virtual void ReadPendingOfInstantiationsForConstexprEntity( + const NamedDecl *D, llvm::SmallSetVector<NamedDecl *, 4> &Decls){}; + /// Read the set of late parsed template functions for this source. /// /// The external source should insert its own late parsed template functions diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h index 2bf91cb5212c5eb..25c200a0bc33c0e 100644 --- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h +++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h @@ -319,6 +319,9 @@ class MultiplexExternalSemaSource : public ExternalSemaSource { void ReadPendingInstantiations( SmallVectorImpl<std::pair<ValueDecl*, SourceLocation> >& Pending) override; + virtual void ReadPendingOfInstantiationsForConstexprEntity( + const NamedDecl *D, llvm::SmallSetVector<NamedDecl *, 4> &Decls) override; + /// Read the set of late parsed template functions for this source. /// /// The external source should insert its own late parsed template functions diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index fdd64f2abbe9375..08642889b0cf3eb 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -695,6 +695,10 @@ enum ASTRecordTypes { /// Record code for an unterminated \#pragma clang assume_nonnull begin /// recorded in a preamble. PP_ASSUME_NONNULL_LOC = 67, + + /// Record code for constexpr templated entities that have been used but not + /// yet instantiated. + PENDING_INSTANTIATIONS_OF_CONSTEXPR_ENTITIES = 68, }; /// Record types used within a source manager block. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 7eefdca6815cdad..358b09ac3e4aa3e 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -814,6 +814,9 @@ class ASTReader /// is the instantiation location. SmallVector<serialization::DeclID, 64> PendingInstantiations; + llvm::DenseMap<serialization::DeclID, std::set<serialization::DeclID>> + PendingInstantiationsOfConstexprEntities; + //@} /// \name DiagnosticsEngine-relevant special data @@ -2101,6 +2104,9 @@ class ASTReader SmallVectorImpl<std::pair<ValueDecl *, SourceLocation>> &Pending) override; + virtual void ReadPendingOfInstantiationsForConstexprEntity( + const NamedDecl *D, llvm::SmallSetVector<NamedDecl *, 4> &Decls) override; + void ReadLateParsedTemplates( llvm::MapVector<const FunctionDecl *, std::unique_ptr<LateParsedTemplate>> &LPTMap) override; diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp index 058e22cb2b814e6..e5b06184d9867cc 100644 --- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp +++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp @@ -310,6 +310,12 @@ void MultiplexExternalSemaSource::ReadPendingInstantiations( Sources[i]->ReadPendingInstantiations(Pending); } +void MultiplexExternalSemaSource::ReadPendingOfInstantiationsForConstexprEntity( + const NamedDecl *D, llvm::SmallSetVector<NamedDecl *, 4> &Decls) { + for (size_t i = 0; i < Sources.size(); ++i) + Sources[i]->ReadPendingOfInstantiationsForConstexprEntity(D, Decls); +}; + void MultiplexExternalSemaSource::ReadLateParsedTemplates( llvm::MapVector<const FunctionDecl *, std::unique_ptr<LateParsedTemplate>> &LPTMap) { diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 9afbf139a1c2b88..afddb156ab34593 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -6487,15 +6487,25 @@ void Sema::PerformPendingInstantiations(bool LocalOnly) { // see CWG2497. void Sema::InstantiateFunctionTemplateSpecializations( SourceLocation PointOfInstantiation, FunctionDecl *Tpl) { + + auto InstantiateAll = [&](const auto &Range) { + for (NamedDecl *Fun : Range) { + InstantiateFunctionDefinition(PointOfInstantiation, + cast<FunctionDecl>(Fun)); + } + }; + auto It = PendingInstantiationsOfConstexprEntities.find(Tpl->getCanonicalDecl()); - if (It == PendingInstantiationsOfConstexprEntities.end()) - return; - for (NamedDecl *Fun : It->second) { - InstantiateFunctionDefinition(PointOfInstantiation, - cast<FunctionDecl>(Fun)); + if (It != PendingInstantiationsOfConstexprEntities.end()) { + InstantiateAll(It->second); + } + + llvm::SmallSetVector<NamedDecl *, 4> Decls; + if (ExternalSource) { + ExternalSource->ReadPendingOfInstantiationsForConstexprEntity(Tpl, Decls); + InstantiateAll(Decls); } - PendingInstantiationsOfConstexprEntities.erase(It); } void Sema::PerformDependentDiagnostics(const DeclContext *Pattern, diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index f22da838424b415..bf65ac2d607dce9 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3709,6 +3709,19 @@ llvm::Error ASTReader::ReadASTBlock(ModuleFile &F, } break; + case PENDING_INSTANTIATIONS_OF_CONSTEXPR_ENTITIES: + if (Record.size() % 2 != 0) + return llvm::createStringError( + std::errc::illegal_byte_sequence, + "Invalid PENDING_INSTANTIATIONS_OF_CONSTEXPR_ENTITIES block"); + + for (unsigned I = 0, N = Record.size(); I != N; /* in loop */) { + DeclID Key = getGlobalDeclID(F, Record[I++]); + DeclID Value = getGlobalDeclID(F, Record[I++]); + PendingInstantiationsOfConstexprEntities[Key].insert(Value); + } + break; + case SEMA_DECL_REFS: if (Record.size() != 3) return llvm::createStringError(std::errc::illegal_byte_sequence, @@ -8718,6 +8731,18 @@ void ASTReader::ReadPendingInstantiations( PendingInstantiations.clear(); } +void ASTReader::ReadPendingOfInstantiationsForConstexprEntity( + const NamedDecl *D, llvm::SmallSetVector<NamedDecl *, 4> &Decls) { + for (auto *Redecl : D->redecls()) { + DeclID Id = Redecl->getGlobalID(); + auto It = PendingInstantiationsOfConstexprEntities.find(Id); + if (It == PendingInstantiationsOfConstexprEntities.end()) + continue; + for (DeclID InstantiationId : It->second) + Decls.insert(cast<NamedDecl>(GetDecl(InstantiationId))); + } +} + void ASTReader::ReadLateParsedTemplates( llvm::MapVector<const FunctionDecl *, std::unique_ptr<LateParsedTemplate>> &LPTMap) { diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 6df815234e235fb..c6ad3bd725af3a7 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -849,6 +849,7 @@ void ASTWriter::WriteBlockInfoBlock() { RECORD(SEMA_DECL_REFS); RECORD(WEAK_UNDECLARED_IDENTIFIERS); RECORD(PENDING_IMPLICIT_INSTANTIATIONS); + RECORD(PENDING_INSTANTIATIONS_OF_CONSTEXPR_ENTITIES); RECORD(UPDATE_VISIBLE); RECORD(DECL_UPDATE_OFFSETS); RECORD(DECL_UPDATES); @@ -4836,6 +4837,16 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot, assert(SemaRef.PendingLocalImplicitInstantiations.empty() && "There are local ones at end of translation unit!"); + // Build a record containing all of pending instantiations of constexpr + // entities. + RecordData PendingInstantiationsOfConstexprEntities; + for (const auto &I : SemaRef.PendingInstantiationsOfConstexprEntities) { + for (const auto &Elem : I.second) { + AddDeclRef(I.first, PendingInstantiationsOfConstexprEntities); + AddDeclRef(Elem, PendingInstantiationsOfConstexprEntities); + } + } + // Build a record containing some declaration references. RecordData SemaDeclRefs; if (SemaRef.StdNamespace || SemaRef.StdBadAlloc || SemaRef.StdAlignValT) { @@ -5153,6 +5164,11 @@ ASTFileSignature ASTWriter::WriteASTCore(Sema &SemaRef, StringRef isysroot, if (!PendingInstantiations.empty()) Stream.EmitRecord(PENDING_IMPLICIT_INSTANTIATIONS, PendingInstantiations); + // Write the record containing pending instantiations of constexpr entities. + if (!PendingInstantiationsOfConstexprEntities.empty()) + Stream.EmitRecord(PENDING_INSTANTIATIONS_OF_CONSTEXPR_ENTITIES, + PendingInstantiationsOfConstexprEntities); + // Write the record containing declaration references of Sema. if (!SemaDeclRefs.empty()) Stream.EmitRecord(SEMA_DECL_REFS, SemaDeclRefs); diff --git a/clang/test/PCH/instantiate-used-constexpr-function.cpp b/clang/test/PCH/instantiate-used-constexpr-function.cpp new file mode 100644 index 000000000000000..3930d0467d866be --- /dev/null +++ b/clang/test/PCH/instantiate-used-constexpr-function.cpp @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -std=c++2a -emit-pch %s -o %t +// RUN: %clang_cc1 -std=c++2a -include-pch %t -verify %s + +// expected-no-diagnostics + +#ifndef HEADER +#define HEADER + +template<typename T> constexpr T f(); +constexpr int g() { return f<int>(); } // #1 + +#else /*included pch*/ + +template<typename T> constexpr T f() { return 123; } +int k[g()]; + +#endif // HEADER _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits