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 <zyn7...@gmail.com> Date: Wed, 2 Apr 2025 14:44:40 +0800 Subject: [PATCH 1/4] [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 <zyn7...@gmail.com> Date: Wed, 2 Apr 2025 19:38:09 +0800 Subject: [PATCH 2/4] 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 <zyn7...@gmail.com> Date: Thu, 3 Apr 2025 00:29:26 +0800 Subject: [PATCH 3/4] 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 <zyn7...@gmail.com> Date: Thu, 3 Apr 2025 14:45:10 +0800 Subject: [PATCH 4/4] 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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits