[clang] [Clang] Fix dependent local class instantiation bugs (PR #134038)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 3 01:25: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/5] [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/5] 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
>From e17ad2e684a6d5c372f8c5111fa62a917d6a597a Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 3 Apr 2025 00:29:26 +0800
Subject: [PATCH 3/5] Don't skip past declaration instantiation
---
clang/lib/Sema/SemaTemplateInstantiate.cpp | 4 ----
1 file changed, 4 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index a46c8c7950710..5b502b494e410 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -4264,10 +4264,6 @@ Sema::InstantiateClassMembers(SourceLocation PointOfInstantiation,
if (FunctionDecl *Pattern =
Function->getInstantiatedFromMemberFunction()) {
- if (!Instantiation->getDeclContext()->isDependentContext() &&
- Function->isIneligibleOrNotSelected())
- continue;
-
if (Function->getTrailingRequiresClause()) {
ConstraintSatisfaction Satisfaction;
if (CheckFunctionConstraints(Function, Satisfaction) ||
>From 5aa9923638de93fedfbac1d8ea7f8677a88daaaf Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 3 Apr 2025 14:45:10 +0800
Subject: [PATCH 4/5] Add tests
---
.../SemaTemplate/instantiate-local-class.cpp | 74 ++++++++++++++++++-
1 file changed, 73 insertions(+), 1 deletion(-)
diff --git a/clang/test/SemaTemplate/instantiate-local-class.cpp b/clang/test/SemaTemplate/instantiate-local-class.cpp
index 298233739900f..bd6ef84a2bc0a 100644
--- a/clang/test/SemaTemplate/instantiate-local-class.cpp
+++ b/clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -1,6 +1,9 @@
// RUN: %clang_cc1 -verify -std=c++11 %s
// RUN: %clang_cc1 -verify -std=c++11 -fdelayed-template-parsing %s
// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++17 %s -DCodeGen -emit-llvm -triple %itanium_abi_triple -o - | FileCheck %s -dump-input=always
+
+#ifndef CodeGen
template<typename T>
void f0() {
@@ -60,7 +63,7 @@ namespace PR8801 {
class X;
typedef int (X::*pmf_type)();
class X : public T { };
-
+
pmf_type pmf = &T::foo;
}
@@ -535,3 +538,72 @@ void foo() {
} // namespace local_recursive_lambda
#endif
+
+#endif // CodeGen
+
+#ifdef CodeGen
+
+namespace LambdaContainingLocalClasses {
+
+template <typename F>
+void GH59734() {
+ [&](auto param) {
+ struct Guard {
+ Guard() {
+ // Check that we're able to create DeclRefExpr to param at this point.
+ static_assert(__is_same(decltype(param), int), "");
+ }
+ ~Guard() {
+ static_assert(__is_same(decltype(param), int), "");
+ }
+ operator decltype(param)() {
+ return decltype(param)();
+ }
+ };
+ Guard guard;
+ param = guard;
+ }(42);
+}
+
+// Guard::Guard():
+// CHECK-DAG: define {{.*}} @_ZZZN28LambdaContainingLocalClasses7GH59734IiEEvvENKUlT_E_clIiEEDaS1_EN5GuardC2Ev
+// Guard::operator int():
+// CHECK-DAG: define {{.*}} @_ZZZN28LambdaContainingLocalClasses7GH59734IiEEvvENKUlT_E_clIiEEDaS1_EN5GuardcviEv
+// Guard::~Guard():
+// CHECK-DAG: define {{.*}} @_ZZZN28LambdaContainingLocalClasses7GH59734IiEEvvENKUlT_E_clIiEEDaS1_EN5GuardD2Ev
+
+struct S {};
+
+template <class T = void>
+auto GH132208 = [](auto param) {
+ struct OnScopeExit {
+ OnScopeExit() {
+ static_assert(__is_same(decltype(param), S), "");
+ }
+ ~OnScopeExit() {
+ static_assert(__is_same(decltype(param), S), "");
+ }
+ operator decltype(param)() {
+ return decltype(param)();
+ }
+ } pending;
+
+ param = pending;
+};
+
+void bar() {
+ GH59734<int>();
+
+ GH132208<void>(S{});
+}
+
+// OnScopeExit::OnScopeExit():
+// CHECK-DAG: define {{.*}} @_ZZNK28LambdaContainingLocalClasses8GH132208IvEMUlT_E_clINS_1SEEEDaS2_EN11OnScopeExitC2Ev
+// OnScopeExit::operator S():
+// CHECK-DAG: define {{.*}} @_ZZNK28LambdaContainingLocalClasses8GH132208IvEMUlT_E_clINS_1SEEEDaS2_EN11OnScopeExitcvS5_Ev
+// OnScopeExit::~OnScopeExit():
+// CHECK-DAG: define {{.*}} @_ZZNK28LambdaContainingLocalClasses8GH132208IvEMUlT_E_clINS_1SEEEDaS2_EN11OnScopeExitD2Ev
+
+} // namespace LambdaContainingLocalClasses
+
+#endif
>From bded73e871d39fc7a25c760ddb96efa88216f93e Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 3 Apr 2025 16:24:33 +0800
Subject: [PATCH 5/5] Nitpicks
---
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 21 +++++++++----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index a8d23fb2be6d5..ab361c2a0f854 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5592,9 +5592,10 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
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.
+ // point to the definition.
//
// 2. That getNameInfo().getSourceRange() might return the TypeLocInfo's
// location it tracked.
@@ -5609,7 +5610,7 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
// 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 = [&] {
+ auto NameLocPointsToPattern = [&] {
DeclarationNameInfo PatternName = PatternDecl->getNameInfo();
DeclarationNameLoc PatternNameLoc = PatternName.getInfo();
switch (PatternName.getName().getNameKind()) {
@@ -5620,18 +5621,17 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
default:
// Cases where DeclarationNameLoc doesn't matter, as it merely contains a
// source range.
- Function->setDeclarationNameLoc(PatternNameLoc);
- return;
+ return PatternNameLoc;
}
TypeSourceInfo *TSI = Function->getNameInfo().getNamedTypeInfo();
- // !!! When could TSI be null?
+ // TSI might be null if the function is named by a constructor template id.
+ // E.g. S<T>() {} for class template S with a template parameter T.
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;
+ return PatternNameLoc;
}
QualType InstT = TSI->getType();
@@ -5641,11 +5641,10 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
TLB.pushTrivial(
Context, InstT,
PatternNameLoc.getNamedTypeInfo()->getTypeLoc().getBeginLoc());
- Function->setDeclarationNameLoc(DeclarationNameLoc::makeNamedTypeLoc(
- TLB.getTypeSourceInfo(Context, InstT)));
+ return DeclarationNameLoc::makeNamedTypeLoc(
+ TLB.getTypeSourceInfo(Context, InstT));
};
-
- CorrectNameLoc();
+ Function->setDeclarationNameLoc(NameLocPointsToPattern());
EnterExpressionEvaluationContext EvalContext(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
More information about the cfe-commits
mailing list