[clang] [Clang] Fix dependent local class instantiation bugs (PR #134038)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 2 04:39:01 PDT 2025
https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/134038
>From a670287721da08e54e2908e9abe52ad86a92769b Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 2 Apr 2025 14:44:40 +0800
Subject: [PATCH 1/2] [Clang] Fix dependent local class instantiation bugs
---
clang/docs/ReleaseNotes.rst | 1 +
clang/lib/Sema/SemaTemplateInstantiate.cpp | 3 ++-
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 18 +++++++++++++++++-
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e409f206f6eae..6107fd7667306 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -348,6 +348,7 @@ Bug Fixes to C++ Support
by template argument deduction.
- Clang is now better at instantiating the function definition after its use inside
of a constexpr lambda. (#GH125747)
+- Fixed a local class member function instantiation bug inside dependent lambdas. (#GH59734), (#GH132208)
- Clang no longer crashes when trying to unify the types of arrays with
certain differences in qualifiers (this could happen during template argument
deduction or when building a ternary operator). (#GH97005)
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 00dcadb41e8fb..a46c8c7950710 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -4264,7 +4264,8 @@ Sema::InstantiateClassMembers(SourceLocation PointOfInstantiation,
if (FunctionDecl *Pattern =
Function->getInstantiatedFromMemberFunction()) {
- if (Function->isIneligibleOrNotSelected())
+ if (!Instantiation->getDeclContext()->isDependentContext() &&
+ Function->isIneligibleOrNotSelected())
continue;
if (Function->getTrailingRequiresClause()) {
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 8aaaea0bcdd66..2dc199dc8cbc2 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5590,7 +5590,6 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
Function->setLocation(PatternDecl->getLocation());
Function->setInnerLocStart(PatternDecl->getInnerLocStart());
Function->setRangeEnd(PatternDecl->getEndLoc());
- Function->setDeclarationNameLoc(PatternDecl->getNameInfo().getInfo());
EnterExpressionEvaluationContext EvalContext(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
@@ -5713,6 +5712,23 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
MultiLevelTemplateArgumentList TemplateArgs = getTemplateInstantiationArgs(
Function, DC, /*Final=*/false, Innermost, false, PatternDecl);
+ // Let the instantiation use the Pattern's DeclarationNameInfo, due to the
+ // following awkwards:
+ // 1. The function we're instantiating might come from an (implicit)
+ // declaration, while the pattern comes from a definition.
+ // 2. We want the instantiated definition to have a source range pointing to
+ // the definition. Note that we can't set the pattern's DeclarationNameInfo
+ // blindly, as it might contain associated TypeLocs that should have
+ // already been transformed. So we transform the Pattern's DNI for that
+ // purpose. Technically, we should create a new function declaration and
+ // give it everything we want, but InstantiateFunctionDefinition does update
+ // the declaration in place.
+ DeclarationNameInfo DN =
+ SubstDeclarationNameInfo(PatternDecl->getNameInfo(), TemplateArgs);
+ if (!DN.getName())
+ return;
+ Function->setDeclarationNameLoc(DN.getInfo());
+
// Substitute into the qualifier; we can get a substitution failure here
// through evil use of alias templates.
// FIXME: Is CurContext correct for this? Should we go to the (instantiation
>From 5bd8347866850458fffbaa1713fe991d82d17bd0 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 2 Apr 2025 19:38:09 +0800
Subject: [PATCH 2/2] Rewrite everything
---
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 73 ++++++++++++++-----
1 file changed, 56 insertions(+), 17 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 2dc199dc8cbc2..a8d23fb2be6d5 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5590,6 +5590,62 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
Function->setLocation(PatternDecl->getLocation());
Function->setInnerLocStart(PatternDecl->getInnerLocStart());
Function->setRangeEnd(PatternDecl->getEndLoc());
+ // Let the instantiation use the Pattern's DeclarationNameLoc, due to the
+ // following awkwardness:
+ // 1. There are out-of-tree users of getNameInfo().getSourceRange(), who
+ // expect the source range of the instantiated declaration to be set to
+ // point the definition.
+ //
+ // 2. That getNameInfo().getSourceRange() might return the TypeLocInfo's
+ // location it tracked.
+ //
+ // 3. Function might come from an (implicit) declaration, while the pattern
+ // comes from a definition. In these cases, we need the PatternDecl's source
+ // location.
+ //
+ // To that end, we need to more or less tweak the DeclarationNameLoc. However,
+ // we can't blindly copy the DeclarationNameLoc from the PatternDecl to the
+ // function, since it contains associated TypeLocs that should have already
+ // been transformed. So, we rebuild the TypeLoc for that purpose. Technically,
+ // we should create a new function declaration and assign everything we need,
+ // but InstantiateFunctionDefinition updates the declaration in place.
+ auto CorrectNameLoc = [&] {
+ DeclarationNameInfo PatternName = PatternDecl->getNameInfo();
+ DeclarationNameLoc PatternNameLoc = PatternName.getInfo();
+ switch (PatternName.getName().getNameKind()) {
+ case DeclarationName::CXXConstructorName:
+ case DeclarationName::CXXDestructorName:
+ case DeclarationName::CXXConversionFunctionName:
+ break;
+ default:
+ // Cases where DeclarationNameLoc doesn't matter, as it merely contains a
+ // source range.
+ Function->setDeclarationNameLoc(PatternNameLoc);
+ return;
+ }
+
+ TypeSourceInfo *TSI = Function->getNameInfo().getNamedTypeInfo();
+ // !!! When could TSI be null?
+ if (!TSI) {
+ // We don't care about the DeclarationName of the instantiated function,
+ // but only the DeclarationNameLoc. So if the TypeLoc is absent, we do
+ // nothing.
+ Function->setDeclarationNameLoc(PatternNameLoc);
+ return;
+ }
+
+ QualType InstT = TSI->getType();
+ // We want to use a TypeLoc that reflects the transformed type while
+ // preserving the source location from the pattern.
+ TypeLocBuilder TLB;
+ TLB.pushTrivial(
+ Context, InstT,
+ PatternNameLoc.getNamedTypeInfo()->getTypeLoc().getBeginLoc());
+ Function->setDeclarationNameLoc(DeclarationNameLoc::makeNamedTypeLoc(
+ TLB.getTypeSourceInfo(Context, InstT)));
+ };
+
+ CorrectNameLoc();
EnterExpressionEvaluationContext EvalContext(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
@@ -5712,23 +5768,6 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
MultiLevelTemplateArgumentList TemplateArgs = getTemplateInstantiationArgs(
Function, DC, /*Final=*/false, Innermost, false, PatternDecl);
- // Let the instantiation use the Pattern's DeclarationNameInfo, due to the
- // following awkwards:
- // 1. The function we're instantiating might come from an (implicit)
- // declaration, while the pattern comes from a definition.
- // 2. We want the instantiated definition to have a source range pointing to
- // the definition. Note that we can't set the pattern's DeclarationNameInfo
- // blindly, as it might contain associated TypeLocs that should have
- // already been transformed. So we transform the Pattern's DNI for that
- // purpose. Technically, we should create a new function declaration and
- // give it everything we want, but InstantiateFunctionDefinition does update
- // the declaration in place.
- DeclarationNameInfo DN =
- SubstDeclarationNameInfo(PatternDecl->getNameInfo(), TemplateArgs);
- if (!DN.getName())
- return;
- Function->setDeclarationNameLoc(DN.getInfo());
-
// Substitute into the qualifier; we can get a substitution failure here
// through evil use of alias templates.
// FIXME: Is CurContext correct for this? Should we go to the (instantiation
More information about the cfe-commits
mailing list