[clang] Reapply "[Clang] Fix dependent local class instantiation bugs" (PR #135914)
Younan Zhang via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 16 18:55:49 PDT 2025
https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/135914
>From 7d39d1a66c171bc6e44742c0baea5bcab777bacd Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 16 Apr 2025 13:27:54 +0800
Subject: [PATCH 1/3] Reapply "[Clang] Fix dependent local class instantiation
bugs"
---
clang/docs/ReleaseNotes.rst | 1 +
clang/lib/Sema/SemaTemplateInstantiate.cpp | 3 -
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 56 +++++++++++++++-
.../CodeGenCXX/local-class-instantiation.cpp | 64 +++++++++++++++++++
4 files changed, 120 insertions(+), 4 deletions(-)
create mode 100644 clang/test/CodeGenCXX/local-class-instantiation.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 84ad253c1ec4f..87c2f72d20a46 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -458,6 +458,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 d2408a94ad0ab..0e81804f8c1e7 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -4126,9 +4126,6 @@ Sema::InstantiateClassMembers(SourceLocation PointOfInstantiation,
if (FunctionDecl *Pattern =
Function->getInstantiatedFromMemberFunction()) {
- if (Function->isIneligibleOrNotSelected())
- continue;
-
if (Function->getTrailingRequiresClause()) {
ConstraintSatisfaction Satisfaction;
if (CheckFunctionConstraints(Function, Satisfaction) ||
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 5c80077f294c6..bf5a882ba4f12 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5597,7 +5597,61 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
Function->setLocation(PatternDecl->getLocation());
Function->setInnerLocStart(PatternDecl->getInnerLocStart());
Function->setRangeEnd(PatternDecl->getEndLoc());
- Function->setDeclarationNameLoc(PatternDecl->getNameInfo().getInfo());
+ // 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 to 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 NameLocPointsToPattern = [&] {
+ 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.
+ return PatternNameLoc;
+ }
+
+ TypeSourceInfo *TSI = Function->getNameInfo().getNamedTypeInfo();
+ // 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.
+ return PatternNameLoc;
+ }
+
+ 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());
+ return DeclarationNameLoc::makeNamedTypeLoc(
+ TLB.getTypeSourceInfo(Context, InstT));
+ };
+ Function->setDeclarationNameLoc(NameLocPointsToPattern());
EnterExpressionEvaluationContext EvalContext(
*this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
diff --git a/clang/test/CodeGenCXX/local-class-instantiation.cpp b/clang/test/CodeGenCXX/local-class-instantiation.cpp
new file mode 100644
index 0000000000000..34103a1ee55ef
--- /dev/null
+++ b/clang/test/CodeGenCXX/local-class-instantiation.cpp
@@ -0,0 +1,64 @@
+// RUN: %clang_cc1 -std=c++17 %s -emit-llvm -triple %itanium_abi_triple -o - | FileCheck %s
+
+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
>From 43876df2b78bcef474d4eeea95b87f0b81c55eca Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Wed, 16 Apr 2025 13:29:41 +0800
Subject: [PATCH 2/3] Ensure the presence of TypeSourceInfo for out-of-line
dependent definition
---
clang/lib/Sema/SemaExprCXX.cpp | 2 +-
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 6 ++--
.../SemaTemplate/instantiate-local-class.cpp | 34 +++++++++++++++++++
3 files changed, 38 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 8df590fa624cf..f83c05ac053ed 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -347,7 +347,7 @@ ParsedType Sema::getDestructorName(const IdentifierInfo &II,
QualType T =
CheckTypenameType(ElaboratedTypeKeyword::None, SourceLocation(),
SS.getWithLocInContext(Context), II, NameLoc);
- return ParsedType::make(T);
+ return CreateParsedType(T, Context.getTrivialTypeSourceInfo(T, NameLoc));
}
// The remaining cases are all non-standard extensions imitating the behavior
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index bf5a882ba4f12..5f370c933f834 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5645,9 +5645,9 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
// 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());
+ TypeSourceInfo *PatternTSI = PatternName.getNamedTypeInfo();
+ assert(PatternTSI && "Pattern is supposed to have an associated TSI");
+ TLB.pushTrivial(Context, InstT, PatternTSI->getTypeLoc().getBeginLoc());
return DeclarationNameLoc::makeNamedTypeLoc(
TLB.getTypeSourceInfo(Context, InstT));
};
diff --git a/clang/test/SemaTemplate/instantiate-local-class.cpp b/clang/test/SemaTemplate/instantiate-local-class.cpp
index 298233739900f..810ce9329eae7 100644
--- a/clang/test/SemaTemplate/instantiate-local-class.cpp
+++ b/clang/test/SemaTemplate/instantiate-local-class.cpp
@@ -535,3 +535,37 @@ void foo() {
} // namespace local_recursive_lambda
#endif
+
+namespace PR134038_Regression {
+
+template <class T> class G {
+public:
+ template <class> class Iter {
+ public:
+ Iter();
+ ~Iter();
+
+ operator G<T>();
+ };
+};
+
+template <class ObserverType>
+template <class ContainerType>
+G<ObserverType>::Iter<ContainerType>::Iter() {}
+
+template <class ObserverType>
+template <class ContainerType>
+G<ObserverType>::Iter<ContainerType>::~Iter() {}
+
+template <class ObserverType>
+template <class ContainerType>
+G<ObserverType>::Iter<ContainerType>::operator G<ObserverType>() {
+ return G<ObserverType>{};
+}
+
+void NotifySettingChanged() {
+ G<int>::Iter<int> Iter;
+ G<int> g = Iter;
+}
+
+}
>From 0fa404f3d31c9d74e0b2b9dbe3c572d23e2ff5c6 Mon Sep 17 00:00:00 2001
From: Younan Zhang <zyn7109 at gmail.com>
Date: Thu, 17 Apr 2025 09:54:47 +0800
Subject: [PATCH 3/3] Use the TypeLocInfo overload of CheckTypenameType
---
clang/lib/Sema/SemaExprCXX.cpp | 6 ++++--
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 3 +++
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index f83c05ac053ed..394e56c465c24 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -344,10 +344,12 @@ ParsedType Sema::getDestructorName(const IdentifierInfo &II,
// We didn't find our type, but that's OK: it's dependent anyway.
// FIXME: What if we have no nested-name-specifier?
+ TypeSourceInfo *TSI = nullptr;
QualType T =
CheckTypenameType(ElaboratedTypeKeyword::None, SourceLocation(),
- SS.getWithLocInContext(Context), II, NameLoc);
- return CreateParsedType(T, Context.getTrivialTypeSourceInfo(T, NameLoc));
+ SS.getWithLocInContext(Context), II, NameLoc, &TSI,
+ /*DeducedTSTContext=*/true);
+ return CreateParsedType(T, TSI);
}
// The remaining cases are all non-standard extensions imitating the behavior
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 5f370c933f834..76c055d28f091 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5647,6 +5647,9 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
TypeLocBuilder TLB;
TypeSourceInfo *PatternTSI = PatternName.getNamedTypeInfo();
assert(PatternTSI && "Pattern is supposed to have an associated TSI");
+ // FIXME: PatternTSI is not trivial. We should copy the source location
+ // along the TypeLoc chain. However a trivial TypeLoc is sufficient for
+ // getNameInfo().getSourceRange().
TLB.pushTrivial(Context, InstT, PatternTSI->getTypeLoc().getBeginLoc());
return DeclarationNameLoc::makeNamedTypeLoc(
TLB.getTypeSourceInfo(Context, InstT));
More information about the cfe-commits
mailing list