[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
llvmbot wrote: Failed to cherry-pick: 346077aaa6bef5652a72a2f3d9fc134ea8fc6a5b https://github.com/llvm/llvm-project/actions/runs/13209645800 Please manually backport the fix and push it to your github fork. Once this is done, please create a [pull request](https://github.com/llvm/llvm-project/compare) https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
mizvekov wrote: /cherry-pick 346077aaa6bef5652a72a2f3d9fc134ea8fc6a5b https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
https://github.com/mizvekov milestoned https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
https://github.com/mizvekov closed https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
https://github.com/mizvekov updated
https://github.com/llvm/llvm-project/pull/125266
>From 1282f6a3181fa37e8649e407fabd72893a01103a Mon Sep 17 00:00:00 2001
From: Matheus Izvekov
Date: Sat, 28 Sep 2024 14:28:58 -0300
Subject: [PATCH] [clang] Track function template instantiation from definition
This fixes instantiation of definition for friend function templates,
when the declaration found and the one containing the definition
have different template contexts.
In these cases, the the function declaration corresponding to the
definition is not available; it may not even be instantiated at all.
So this patch adds a bit which tracks which function template
declaration was instantiated from the member template.
It's used to find which primary template serves as a context
for the purpose of obtainining the template arguments needed
to instantiate the definition.
Fixes #55509
---
clang/docs/ReleaseNotes.rst | 2 +
clang/include/clang/AST/Decl.h| 7 ++
clang/include/clang/AST/DeclBase.h| 10 +-
clang/include/clang/AST/DeclTemplate.h| 20
clang/lib/AST/Decl.cpp| 1 +
clang/lib/Sema/SemaTemplateDeduction.cpp | 17 +--
clang/lib/Sema/SemaTemplateInstantiate.cpp| 9 +-
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 27 -
clang/lib/Serialization/ASTReaderDecl.cpp | 1 +
clang/lib/Serialization/ASTWriterDecl.cpp | 3 +-
clang/test/SemaTemplate/GH55509.cpp | 112 ++
11 files changed, 181 insertions(+), 28 deletions(-)
create mode 100644 clang/test/SemaTemplate/GH55509.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f7feca0089f94d..21c1ff25d2862b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -145,6 +145,8 @@ Bug Fixes to Attribute Support
Bug Fixes to C++ Support
+- Clang is now better at keeping track of friend function template instance
contexts. (#GH55509)
+
Bug Fixes to AST Handling
^
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 499d27a9be5a8a..f305cbbce4c609 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
/// Whether this function is "trivial" in some specialized C++ senses.
/// Can only be true for default constructors, copy constructors,
/// copy assignment operators, and destructors. Not meaningful until
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 3bb82c1572ef9c..648dae2838e036 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -1780,6 +1780,8 @@ class DeclContext {
uint64_t HasImplicitReturnZero : 1;
LLVM_PREFERRED_TYPE(bool)
uint64_t IsLateTemplateParsed : 1;
+LLVM_PREFERRED_TYPE(bool)
+uint64_t IsInstantiatedFromMemberTemplate : 1;
/// Kind of contexpr specifier as defined by ConstexprSpecKind.
LLVM_PREFERRED_TYPE(ConstexprSpecKind)
@@ -1830,7 +1832,7 @@ class DeclContext {
};
/// Number of inherited and non-inherited bits in FunctionDeclBitfields.
- enum { NumFunctionDeclBits = NumDeclContextBits + 31 };
+ enum { NumFunctionDeclBits = NumDeclContextBits + 32 };
/// Stores the bits used by CXXConstructorDecl. If modified
/// NumCXXConstructorDeclBits and the accessor
@@ -1841,12 +1843,12 @@ class DeclContext {
LLVM_PREFERRED_TYPE(FunctionDeclBitfields)
uint64_t : NumFunctionDeclBits;
-/// 20 bits to fit in the remaining available space.
+/// 19 bits to fit in the remaining available space.
/// Note that this makes CXXConstructorDeclBitfields take
/// exactly 64 bits and thus the width of NumCtorInitializers
/// will need to be shrunk if some bit is added to NumDeclContextBitfields,
/// NumFunctionDeclBitfields or CXXConstructorDeclBitfields.
-uint64_t NumCtorInitializers : 17;
+uint64_t NumCtorInitializers : 16;
LLVM_PREFERRED_TYPE(bool)
uint64_t IsInheritingConstructor : 1;
@@ -1860,7 +1862,7 @@ class DeclContext {
};
/// Number of inherited and non-inherited bits in
CXXConstructorDeclBitfields.
- enum { NumCXXConstructorDeclBits = NumFunctionDeclBits + 20 };
+ enum { NumCXXConstructorDeclBits = NumFunctionDeclBits + 19 };
/// Stores the bits used by ObjCMethodDecl.
/// If modified NumObjCMethodDeclBits and the accessor
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 87406b0e030df1..a30ae798a99bc6 100644
--- a/clang/incl
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
mizvekov wrote:
Thanks, I have added a further comment explaining more things.
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
cor3ntin wrote:
> These are for different things. getInstantiatedFromDecl is used to tell which
> function this "was instantiated from, if this is a function declaration
> declared locally inside of a function template".
UGH, completely missed that - all of this could use some more comments
> Because FunctionDecl has free bits, so this change has basically no space
> cost :)
Yeah, but 1/it's a bit confusing 2/ bits there are fairly precious
You could add a bit to FunctionTemplateSpecializationInfo::Function
I don't insist on it. However can you add some comments? It took me a while to
understand and I suspect i won;t be the only one.
Thanks, I'll approve when i wake up (and sorry it took me a while to understand
everything)
Oh, another solution would be to have a DenseMap of FunctionTemplateDecl ->
FunctionTemplateDecl in Sema, storing specialization of friend functions
template (only!) in it - hopefully these things are fairly rare - and it might
simplify the change to InstantiateFunctionDefinition
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
mizvekov wrote:
Yes, one possibility would be to move the `InstantiatedFromMemberTemplate`
field from the common area into the redeclarable.
This would also increase source representation accuracy, as we would be able to
point to the exact template redeclaration for each instantiated redeclaration,
but it would waste one pointer per Redeclarable. The Function is the only part
which really requires the extra information for correctness reasons.
We could on a follow up consider that change, but it would have to stand on its
own against the costs, without relying on the bug fix to make it worthwhile.
> However, why is FunctionDecl::getInstantiatedFromDecl /
> FunctionDecl::getMemberSpecializationInfo not what we want?
These are for different things. `getInstantiatedFromDecl` is used to tell which
function this "was instantiated from, if this is a function declaration
declared locally inside of a function template".
`getMemberSpecializationInfo` is info for member functions, but friends are not
members.
> If we can't use that, why are we storing a bit in FunctionDecl and not in
> FunctionTemplateSpecializationInfo for example?
Because FunctionDecl has free bits, so this change has basically no space cost
:)
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
cor3ntin wrote:
I had a chat with @erichkeane - I think I understand a bit more now.
At least I understand why `RedeclarableTemplateDecl` is not suitable
However, why is `FunctionDecl::getInstantiatedFromDecl` /
`FunctionDecl::getMemberSpecializationInfo` not what we want?
If we can't use that, why are we storing a bit in FunctionDecl and not in
`FunctionTemplateSpecializationInfo` for example?
I think we need comments somewhere explaining all of that because It's not
trivial.
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
erichkeane wrote:
I consider we could probably do this tracking more cleanly at the
`RedeclarableTemplateDecl` level (have it track WHICH declaration is the one
that was instantiated), but that both requires modifying the
`RedeclarableTemplateDecl` after it was generated (making it not-const), plus
using 64 bits, instead of 1-per-decl.
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
erichkeane wrote:
This is a 'thanks i hate it' :) Basically, the `RedeclarableTemplateDecl`
applies to the declaration chain, but this is separately tracking on a
per-declaration basis. It is a little frustrating that we have to do this.
But I think we're stuck with it unless someone has a better idea
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
mizvekov wrote:
By the way, this was all discussed in the original review:
https://github.com/llvm/llvm-project/pull/110387
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
mizvekov wrote:
```
Note that RedeclarableTemplateDecl::setInstantiatedFromMemberTemplate is
already called everywhere it needs to (hopefully) and
isCompatibleWithDefinition() can just be return
isInstantiatedFromMemberTemplate() || isThisDeclarationADefinition();` - unless
I am missing something, but in that case the PR needs more explanation.
```
The field `RedeclarableTemplateDecl::setInstantiatedFromMemberTemplate` uses is
in the common area, the new flag which is set from the new
`setInstantiatedFromMemberTemplate` is per declaration.
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
https://github.com/mizvekov edited https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
mizvekov wrote:
The new `setInstantiatedFromMemberTemplate` is being used from existing
callers, which before this patch would call this member from a
FunctionTemplateDecl object, which would call
`RedeclarableTemplateDecl::setInstantiatedFromMemberTemplate`.
With this patch, these callers will find
`FunctionTemplateDecl::setInstantiatedFromMemberTemplate` instead.
See the implementation of the latter, where it also forwards the call to the
former.
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
cor3ntin wrote:
`ASTReader / Writer.`, sure.
However, I don't understand where `IsInstantiatedFromMemberTemplate` is
initially set
(and i don't understand why we would need it - we already store that
information on the `FunctionTemplateDecl` - which is what you have in your
changes to `InstantiateFunctionDefinition`.
Go through the different calls to `setInstantiatedFromMemberTemplate` and see
that this is _only_ used in ASTreader, and the new
`FunctionTemplateDecl::setInstantiatedFromMemberTemplate` is not used - unless
I am blind, which is always a possibility at 7am :)
Note that `RedeclarableTemplateDecl::setInstantiatedFromMemberTemplate` is
already called everywhere it needs to (hopefully) and
`isCompatibleWithDefinition()` can just be `return
`isInstantiatedFromMemberTemplate() || isThisDeclarationADefinition();` -
unless I am missing something, but in that case the PR needs more explanation.
>From a FunctionFecl,
>`FunctionDecl::getInstantiatedFromDecl/FunctionDecl::getInstantiatedFromMemberFunction`
> _should_ also have the information
@erichkeane @AaronBallman
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
mizvekov wrote:
Yes, from new methods in DeclTemplate.h, which themselves are used from
SemaTemplateInstantiateDecl.cpp and from the ASTReader / Writer.
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
https://github.com/cor3ntin edited https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
cor3ntin wrote:
Is that even used?
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -5276,9 +5277,31 @@ void Sema::InstantiateFunctionDefinition(SourceLocation
PointOfInstantiation,
RebuildTypeSourceInfoForDefaultSpecialMembers();
SetDeclDefaulted(Function, PatternDecl->getLocation());
} else {
+NamedDecl *ND = Function;
+DeclContext *DC = ND->getLexicalDeclContext();
+std::optional> Innermost;
+if (auto *Primary = Function->getPrimaryTemplate();
+Primary &&
+!isGenericLambdaCallOperatorOrStaticInvokerSpecialization(Function) &&
+Function->getTemplateSpecializationKind() !=
+TSK_ExplicitSpecialization) {
+ auto It = llvm::find_if(Primary->redecls(),
+ [](const RedeclarableTemplateDecl *RTD) {
+return cast(RTD)
+->isCompatibleWithDefinition();
+ });
+ assert(It != Primary->redecls().end() &&
+ "Should't get here without a definition");
+ if (FunctionDecl *Def = cast(*It)
+ ->getTemplatedDecl()
+ ->getDefinition())
+DC = Def->getLexicalDeclContext();
+ else
+DC = (*It)->getLexicalDeclContext();
mizvekov wrote:
Well, there is commonality in that both are using the same helpers, but
`getTemplateInstantiationPattern(true)` doesn't do what we need here, and this
bit doesn't do what the other users of `getTemplateInstantiationPattern(true)`
need. I am not sure it would be worth joining both implementations, adding an
extra parameter to `getTemplateInstantiationPattern`.
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -5276,9 +5277,31 @@ void Sema::InstantiateFunctionDefinition(SourceLocation
PointOfInstantiation,
RebuildTypeSourceInfoForDefaultSpecialMembers();
SetDeclDefaulted(Function, PatternDecl->getLocation());
} else {
+NamedDecl *ND = Function;
+DeclContext *DC = ND->getLexicalDeclContext();
+std::optional> Innermost;
+if (auto *Primary = Function->getPrimaryTemplate();
+Primary &&
+!isGenericLambdaCallOperatorOrStaticInvokerSpecialization(Function) &&
+Function->getTemplateSpecializationKind() !=
+TSK_ExplicitSpecialization) {
+ auto It = llvm::find_if(Primary->redecls(),
+ [](const RedeclarableTemplateDecl *RTD) {
+return cast(RTD)
+->isCompatibleWithDefinition();
+ });
+ assert(It != Primary->redecls().end() &&
+ "Should't get here without a definition");
+ if (FunctionDecl *Def = cast(*It)
+ ->getTemplatedDecl()
+ ->getDefinition())
+DC = Def->getLexicalDeclContext();
+ else
+DC = (*It)->getLexicalDeclContext();
cor3ntin wrote:
Sorry, nevermind. I understand what you are doing now.
However, I'm wondering if we are not reinventing
`getTemplateInstantiationPattern(true)` here, It looks strangely similar. Have
you explored that?
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -5276,9 +5277,31 @@ void Sema::InstantiateFunctionDefinition(SourceLocation
PointOfInstantiation,
RebuildTypeSourceInfoForDefaultSpecialMembers();
SetDeclDefaulted(Function, PatternDecl->getLocation());
} else {
+NamedDecl *ND = Function;
+DeclContext *DC = ND->getLexicalDeclContext();
+std::optional> Innermost;
+if (auto *Primary = Function->getPrimaryTemplate();
+Primary &&
+!isGenericLambdaCallOperatorOrStaticInvokerSpecialization(Function) &&
+Function->getTemplateSpecializationKind() !=
+TSK_ExplicitSpecialization) {
+ auto It = llvm::find_if(Primary->redecls(),
+ [](const RedeclarableTemplateDecl *RTD) {
+return cast(RTD)
+->isCompatibleWithDefinition();
+ });
+ assert(It != Primary->redecls().end() &&
+ "Should't get here without a definition");
+ if (FunctionDecl *Def = cast(*It)
+ ->getTemplatedDecl()
+ ->getDefinition())
+DC = Def->getLexicalDeclContext();
+ else
+DC = (*It)->getLexicalDeclContext();
mizvekov wrote:
We need the lexical context because we are interested in the template context
of the declaration, ie what set of parameters it sits under, which is a lexical
thing.
I don't follow what you mean this situation with friends, can you provide an
example?
regression2 is a situation very specific to member function templates, but
friends are not members.
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
@@ -5276,9 +5277,31 @@ void Sema::InstantiateFunctionDefinition(SourceLocation
PointOfInstantiation,
RebuildTypeSourceInfoForDefaultSpecialMembers();
SetDeclDefaulted(Function, PatternDecl->getLocation());
} else {
+NamedDecl *ND = Function;
+DeclContext *DC = ND->getLexicalDeclContext();
+std::optional> Innermost;
+if (auto *Primary = Function->getPrimaryTemplate();
+Primary &&
+!isGenericLambdaCallOperatorOrStaticInvokerSpecialization(Function) &&
+Function->getTemplateSpecializationKind() !=
+TSK_ExplicitSpecialization) {
+ auto It = llvm::find_if(Primary->redecls(),
+ [](const RedeclarableTemplateDecl *RTD) {
+return cast(RTD)
+->isCompatibleWithDefinition();
+ });
+ assert(It != Primary->redecls().end() &&
+ "Should't get here without a definition");
+ if (FunctionDecl *Def = cast(*It)
+ ->getTemplatedDecl()
+ ->getDefinition())
+DC = Def->getLexicalDeclContext();
+ else
+DC = (*It)->getLexicalDeclContext();
cor3ntin wrote:
Can we get into a situation like `regression2` with friends?
I wonder if we can use `DC = (*It)->getDeclContext();` (not lexical!)
unconditionally.
https://github.com/llvm/llvm-project/pull/125266
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
mizvekov wrote: > I’m not in a position where I can test the patch properly right now, but if > the whole qtbase can be built (not only the reduced testcase or the specific > source file that triggered the issue before), I’m good with it - thanks! Yes it does, no problem! https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
mstorsjo wrote: I’m not in a position where I can test the patch properly right now, but if the whole qtbase can be built (not only the reduced testcase or the specific source file that triggered the issue before), I’m good with it - thanks! https://github.com/llvm/llvm-project/pull/125266 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
llvmbot wrote:
@llvm/pr-subscribers-clang-modules
Author: Matheus Izvekov (mizvekov)
Changes
This fixes instantiation of definition for friend function templates,
when the declaration found and the one containing the definition
have different template contexts.
In these cases, the the function declaration corresponding to the
definition is not available; it may not even be instantiated at all.
So this patch adds a bit which tracks which function template
declaration was instantiated from the member template.
It's used to find which primary template serves as a context
for the purpose of obtainining the template arguments needed
to instantiate the definition.
Fixes https://github.com/llvm/llvm-project/issues/55509
---
Full diff: https://github.com/llvm/llvm-project/pull/125266.diff
11 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+2)
- (modified) clang/include/clang/AST/Decl.h (+7)
- (modified) clang/include/clang/AST/DeclBase.h (+6-4)
- (modified) clang/include/clang/AST/DeclTemplate.h (+9)
- (modified) clang/lib/AST/Decl.cpp (+1)
- (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+1-16)
- (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+4-5)
- (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+25-2)
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+1)
- (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+2-1)
- (added) clang/test/SemaTemplate/GH55509.cpp (+112)
``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8be1ea2fb01455f..500fce9dcb72745 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,8 @@ Bug Fixes to Attribute Support
Bug Fixes to C++ Support
+- Clang is now better at keeping track of friend function template instance
contexts. (#GH55509)
+
Bug Fixes to AST Handling
^
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 499d27a9be5a8a4..f305cbbce4c609a 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
/// Whether this function is "trivial" in some specialized C++ senses.
/// Can only be true for default constructors, copy constructors,
/// copy assignment operators, and destructors. Not meaningful until
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 2c0c3a8dc2f9d5c..3a13309a6100eed 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -1777,6 +1777,8 @@ class DeclContext {
uint64_t HasImplicitReturnZero : 1;
LLVM_PREFERRED_TYPE(bool)
uint64_t IsLateTemplateParsed : 1;
+LLVM_PREFERRED_TYPE(bool)
+uint64_t IsInstantiatedFromMemberTemplate : 1;
/// Kind of contexpr specifier as defined by ConstexprSpecKind.
LLVM_PREFERRED_TYPE(ConstexprSpecKind)
@@ -1827,7 +1829,7 @@ class DeclContext {
};
/// Number of inherited and non-inherited bits in FunctionDeclBitfields.
- enum { NumFunctionDeclBits = NumDeclContextBits + 31 };
+ enum { NumFunctionDeclBits = NumDeclContextBits + 32 };
/// Stores the bits used by CXXConstructorDecl. If modified
/// NumCXXConstructorDeclBits and the accessor
@@ -1838,12 +1840,12 @@ class DeclContext {
LLVM_PREFERRED_TYPE(FunctionDeclBitfields)
uint64_t : NumFunctionDeclBits;
-/// 20 bits to fit in the remaining available space.
+/// 19 bits to fit in the remaining available space.
/// Note that this makes CXXConstructorDeclBitfields take
/// exactly 64 bits and thus the width of NumCtorInitializers
/// will need to be shrunk if some bit is added to NumDeclContextBitfields,
/// NumFunctionDeclBitfields or CXXConstructorDeclBitfields.
-uint64_t NumCtorInitializers : 17;
+uint64_t NumCtorInitializers : 16;
LLVM_PREFERRED_TYPE(bool)
uint64_t IsInheritingConstructor : 1;
@@ -1857,7 +1859,7 @@ class DeclContext {
};
/// Number of inherited and non-inherited bits in
CXXConstructorDeclBitfields.
- enum { NumCXXConstructorDeclBits = NumFunctionDeclBits + 20 };
+ enum { NumCXXConstructorDeclBits = NumFunctionDeclBits + 19 };
/// Stores the bits used by ObjCMethodDecl.
/// If modified NumObjCMethodDeclBits and the accessor
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 9ecff2c898acd5f..0c706036ff70227 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1011,6 +1011,15 @@ class FunctionTemplateDecl : public
RedeclarableTemplateDecl {
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Matheus Izvekov (mizvekov)
Changes
This fixes instantiation of definition for friend function templates,
when the declaration found and the one containing the definition
have different template contexts.
In these cases, the the function declaration corresponding to the
definition is not available; it may not even be instantiated at all.
So this patch adds a bit which tracks which function template
declaration was instantiated from the member template.
It's used to find which primary template serves as a context
for the purpose of obtainining the template arguments needed
to instantiate the definition.
Fixes https://github.com/llvm/llvm-project/issues/55509
---
Full diff: https://github.com/llvm/llvm-project/pull/125266.diff
11 Files Affected:
- (modified) clang/docs/ReleaseNotes.rst (+2)
- (modified) clang/include/clang/AST/Decl.h (+7)
- (modified) clang/include/clang/AST/DeclBase.h (+6-4)
- (modified) clang/include/clang/AST/DeclTemplate.h (+9)
- (modified) clang/lib/AST/Decl.cpp (+1)
- (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+1-16)
- (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+4-5)
- (modified) clang/lib/Sema/SemaTemplateInstantiateDecl.cpp (+25-2)
- (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+1)
- (modified) clang/lib/Serialization/ASTWriterDecl.cpp (+2-1)
- (added) clang/test/SemaTemplate/GH55509.cpp (+112)
``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8be1ea2fb01455f..500fce9dcb72745 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,8 @@ Bug Fixes to Attribute Support
Bug Fixes to C++ Support
+- Clang is now better at keeping track of friend function template instance
contexts. (#GH55509)
+
Bug Fixes to AST Handling
^
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 499d27a9be5a8a4..f305cbbce4c609a 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
/// Whether this function is "trivial" in some specialized C++ senses.
/// Can only be true for default constructors, copy constructors,
/// copy assignment operators, and destructors. Not meaningful until
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 2c0c3a8dc2f9d5c..3a13309a6100eed 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -1777,6 +1777,8 @@ class DeclContext {
uint64_t HasImplicitReturnZero : 1;
LLVM_PREFERRED_TYPE(bool)
uint64_t IsLateTemplateParsed : 1;
+LLVM_PREFERRED_TYPE(bool)
+uint64_t IsInstantiatedFromMemberTemplate : 1;
/// Kind of contexpr specifier as defined by ConstexprSpecKind.
LLVM_PREFERRED_TYPE(ConstexprSpecKind)
@@ -1827,7 +1829,7 @@ class DeclContext {
};
/// Number of inherited and non-inherited bits in FunctionDeclBitfields.
- enum { NumFunctionDeclBits = NumDeclContextBits + 31 };
+ enum { NumFunctionDeclBits = NumDeclContextBits + 32 };
/// Stores the bits used by CXXConstructorDecl. If modified
/// NumCXXConstructorDeclBits and the accessor
@@ -1838,12 +1840,12 @@ class DeclContext {
LLVM_PREFERRED_TYPE(FunctionDeclBitfields)
uint64_t : NumFunctionDeclBits;
-/// 20 bits to fit in the remaining available space.
+/// 19 bits to fit in the remaining available space.
/// Note that this makes CXXConstructorDeclBitfields take
/// exactly 64 bits and thus the width of NumCtorInitializers
/// will need to be shrunk if some bit is added to NumDeclContextBitfields,
/// NumFunctionDeclBitfields or CXXConstructorDeclBitfields.
-uint64_t NumCtorInitializers : 17;
+uint64_t NumCtorInitializers : 16;
LLVM_PREFERRED_TYPE(bool)
uint64_t IsInheritingConstructor : 1;
@@ -1857,7 +1859,7 @@ class DeclContext {
};
/// Number of inherited and non-inherited bits in
CXXConstructorDeclBitfields.
- enum { NumCXXConstructorDeclBits = NumFunctionDeclBits + 20 };
+ enum { NumCXXConstructorDeclBits = NumFunctionDeclBits + 19 };
/// Stores the bits used by ObjCMethodDecl.
/// If modified NumObjCMethodDeclBits and the accessor
diff --git a/clang/include/clang/AST/DeclTemplate.h
b/clang/include/clang/AST/DeclTemplate.h
index 9ecff2c898acd5f..0c706036ff70227 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1011,6 +1011,15 @@ class FunctionTemplateDecl : public
RedeclarableTemplateDecl {
retu
[clang] Reland: [clang] Track function template instantiation from definition (PR #125266)
https://github.com/mizvekov created
https://github.com/llvm/llvm-project/pull/125266
This fixes instantiation of definition for friend function templates,
when the declaration found and the one containing the definition
have different template contexts.
In these cases, the the function declaration corresponding to the
definition is not available; it may not even be instantiated at all.
So this patch adds a bit which tracks which function template
declaration was instantiated from the member template.
It's used to find which primary template serves as a context
for the purpose of obtainining the template arguments needed
to instantiate the definition.
Fixes https://github.com/llvm/llvm-project/issues/55509
>From 2ff73103c3f9cd61d1d829393d940fd95a751932 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov
Date: Sat, 28 Sep 2024 14:28:58 -0300
Subject: [PATCH 1/2] [clang] Track function template instantiation from
definition
This fixes instantiation of definition for friend function templates,
when the declaration found and the one containing the definition
have different template contexts.
In these cases, the the function declaration corresponding to the
definition is not available; it may not even be instantiated at all.
So this patch adds a bit which tracks which function template
declaration was instantiated from the member template.
It's used to find which primary template serves as a context
for the purpose of obtainining the template arguments needed
to instantiate the definition.
Fixes #55509
---
clang/docs/ReleaseNotes.rst | 2 +
clang/include/clang/AST/Decl.h| 7 ++
clang/include/clang/AST/DeclBase.h| 10 +-
clang/include/clang/AST/DeclTemplate.h| 9 ++
clang/lib/AST/Decl.cpp| 1 +
clang/lib/Sema/SemaTemplateDeduction.cpp | 17 +--
clang/lib/Sema/SemaTemplateInstantiate.cpp| 9 +-
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 22 +++-
clang/lib/Serialization/ASTReaderDecl.cpp | 1 +
clang/lib/Serialization/ASTWriterDecl.cpp | 3 +-
clang/test/SemaTemplate/GH55509.cpp | 101 ++
11 files changed, 154 insertions(+), 28 deletions(-)
create mode 100644 clang/test/SemaTemplate/GH55509.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8be1ea2fb01455f..500fce9dcb72745 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,8 @@ Bug Fixes to Attribute Support
Bug Fixes to C++ Support
+- Clang is now better at keeping track of friend function template instance
contexts. (#GH55509)
+
Bug Fixes to AST Handling
^
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 499d27a9be5a8a4..f305cbbce4c609a 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2298,6 +2298,13 @@ class FunctionDecl : public DeclaratorDecl,
FunctionDeclBits.IsLateTemplateParsed = ILT;
}
+ bool isInstantiatedFromMemberTemplate() const {
+return FunctionDeclBits.IsInstantiatedFromMemberTemplate;
+ }
+ void setInstantiatedFromMemberTemplate(bool Val = true) {
+FunctionDeclBits.IsInstantiatedFromMemberTemplate = Val;
+ }
+
/// Whether this function is "trivial" in some specialized C++ senses.
/// Can only be true for default constructors, copy constructors,
/// copy assignment operators, and destructors. Not meaningful until
diff --git a/clang/include/clang/AST/DeclBase.h
b/clang/include/clang/AST/DeclBase.h
index 2c0c3a8dc2f9d5c..3a13309a6100eed 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -1777,6 +1777,8 @@ class DeclContext {
uint64_t HasImplicitReturnZero : 1;
LLVM_PREFERRED_TYPE(bool)
uint64_t IsLateTemplateParsed : 1;
+LLVM_PREFERRED_TYPE(bool)
+uint64_t IsInstantiatedFromMemberTemplate : 1;
/// Kind of contexpr specifier as defined by ConstexprSpecKind.
LLVM_PREFERRED_TYPE(ConstexprSpecKind)
@@ -1827,7 +1829,7 @@ class DeclContext {
};
/// Number of inherited and non-inherited bits in FunctionDeclBitfields.
- enum { NumFunctionDeclBits = NumDeclContextBits + 31 };
+ enum { NumFunctionDeclBits = NumDeclContextBits + 32 };
/// Stores the bits used by CXXConstructorDecl. If modified
/// NumCXXConstructorDeclBits and the accessor
@@ -1838,12 +1840,12 @@ class DeclContext {
LLVM_PREFERRED_TYPE(FunctionDeclBitfields)
uint64_t : NumFunctionDeclBits;
-/// 20 bits to fit in the remaining available space.
+/// 19 bits to fit in the remaining available space.
/// Note that this makes CXXConstructorDeclBitfields take
/// exactly 64 bits and thus the width of NumCtorInitializers
/// will need to be shrunk if some bit is added to NumDeclContextBitfields,
/// NumFunctionDeclBitfields or CXXConstructorDeclBitfields.
-uint64_t NumCtorInitializers : 17;
