[Clang][Sema] Fix the lambda call expression inside of a type alias declaration #82310
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
zyn0217
changed the title
|
The Windows CI is still broken and I have to run the libc++ tests locally. |
|
Result for libc++ tests: The failing test seems to be my local configuration issue and doesn't relate to this patch IIUC. |
zyn0217
added
the
clang:frontend
|
@llvm/pr-subscribers-clang Author: Younan Zhang (zyn0217) ChangesThis patch attempts to fix the lambda call expression inside of a type alias declaration from two aspects:
It is unfortunate that a Fixes #70601 Note that this doesn't involve the fix for #23317. That seems different, and I'd like to leave it as a follow-up. Full diff: https://github.com/llvm/llvm-project/pull/82310.diff 10 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 649ad655905af2..7988912faa2075 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -267,6 +267,11 @@ Bug Fixes to C++ Support
was only accepted at namespace scope but not at local function scope.
- Clang no longer tries to call consteval constructors at runtime when they appear in a member initializer.
(`#782154 <https://github.com/llvm/llvm-project/issues/82154>`_`)
+- Clang now supports direct lambda calls inside of a type alias template declarations.
+ This addresses (`#70601 <https://github.com/llvm/llvm-project/issues/70601>`_),
+ (`#76674 <https://github.com/llvm/llvm-project/issues/76674>`_),
+ (`#79555 <https://github.com/llvm/llvm-project/issues/79555>`_),
+ (`#81145 <https://github.com/llvm/llvm-project/issues/81145>`_), and so on.
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h
index 9cebaff63bb0db..7aed4d5cbc002e 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -1869,6 +1869,10 @@ class CXXRecordDecl : public RecordDecl {
DL.MethodTyInfo = TS;
}
+ void setLambdaDependencyKind(unsigned Kind) {
+ getLambdaData().DependencyKind = Kind;
+ }
+
void setLambdaIsGeneric(bool IsGeneric) {
assert(DefinitionData && DefinitionData->IsLambda &&
"setting lambda property of non-lambda class");
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index e9cd42ae777df5..5847b0301a4ca4 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9622,6 +9622,9 @@ class Sema final {
/// We are building deduction guides for a class.
BuildingDeductionGuides,
+
+ /// We are instantiating a type alias template declaration.
+ TypeAliasTemplateInstantiation,
} Kind;
/// Was the enclosing context a non-instantiation SFINAE context?
@@ -9812,6 +9815,12 @@ class Sema final {
FunctionDecl *Entity, ExceptionSpecification,
SourceRange InstantiationRange = SourceRange());
+ /// Note that we are instantiating a type alias template declaration.
+ InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
+ TypeAliasTemplateDecl *Template,
+ ArrayRef<TemplateArgument> TemplateArgs,
+ SourceRange InstantiationRange = SourceRange());
+
/// Note that we are instantiating a default argument in a
/// template-id.
InstantiatingTemplate(Sema &SemaRef, SourceLocation PointOfInstantiation,
diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp
index b9ed5dedfa4223..43d6e2230fb129 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -426,6 +426,8 @@ class DefaultTemplateInstCallback : public TemplateInstantiationCallback {
return "BuildingBuiltinDumpStructCall";
case CodeSynthesisContext::BuildingDeductionGuides:
return "BuildingDeductionGuides";
+ case Sema::CodeSynthesisContext::TypeAliasTemplateInstantiation:
+ return "TypeAliasTemplateInstantiation";
}
return "";
}
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 2878e4d31ee8fe..5908b941f21a7d 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -614,10 +614,14 @@ bool Sema::SetupConstraintScope(
// reference the original primary template.
// We walk up the instantiated template chain so that nested lambdas get
// handled properly.
- for (FunctionTemplateDecl *FromMemTempl =
- PrimaryTemplate->getInstantiatedFromMemberTemplate();
- FromMemTempl;
- FromMemTempl = FromMemTempl->getInstantiatedFromMemberTemplate()) {
+ // Note that we shall not collect instantiated parameters from
+ // 'intermediate' transformed function templates but the primary template
+ // for which we have built up the template arguments relative to. Otherwise,
+ // we may have mismatched template parameter depth!
+ if (FunctionTemplateDecl *FromMemTempl =
+ PrimaryTemplate->getInstantiatedFromMemberTemplate()) {
+ while (FromMemTempl->getInstantiatedFromMemberTemplate())
+ FromMemTempl = FromMemTempl->getInstantiatedFromMemberTemplate();
if (addInstantiatedParametersToScope(FD, FromMemTempl->getTemplatedDecl(),
Scope, MLTAL))
return true;
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 9e516da2aa27a1..85962afb8899e9 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4016,9 +4016,13 @@ QualType Sema::CheckTemplateIdType(TemplateName Name,
if (Inst.isInvalid())
return QualType();
- CanonType = SubstType(Pattern->getUnderlyingType(),
- TemplateArgLists, AliasTemplate->getLocation(),
- AliasTemplate->getDeclName());
+ InstantiatingTemplate InstTemplate(
+ *this, /*PointOfInstantiation=*/AliasTemplate->getBeginLoc(),
+ /*Template=*/AliasTemplate,
+ /*TemplateArgs=*/TemplateArgLists.getInnermost());
+ CanonType =
+ SubstType(Pattern->getUnderlyingType(), TemplateArgLists,
+ AliasTemplate->getLocation(), AliasTemplate->getDeclName());
if (CanonType.isNull()) {
// If this was enable_if and we failed to find the nested type
// within enable_if in a SFINAE context, dig out the specific
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 371378485626c2..a94b48ecd13ffb 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -282,7 +282,7 @@ Response HandleFunctionTemplateDecl(const FunctionTemplateDecl *FTD,
return Response::ChangeDecl(FTD->getLexicalDeclContext());
}
-Response HandleRecordDecl(const CXXRecordDecl *Rec,
+Response HandleRecordDecl(Sema &SemaRef, const CXXRecordDecl *Rec,
MultiLevelTemplateArgumentList &Result,
ASTContext &Context,
bool ForConstraintInstantiation) {
@@ -313,9 +313,75 @@ Response HandleRecordDecl(const CXXRecordDecl *Rec,
// This is to make sure we pick up the VarTemplateSpecializationDecl that this
// lambda is defined inside of.
- if (Rec->isLambda())
+ if (Rec->isLambda()) {
if (const Decl *LCD = Rec->getLambdaContextDecl())
return Response::ChangeDecl(LCD);
+ // Attempt to retrieve the template arguments for a using alias declaration.
+ // This is necessary for constraint checking, since we always keep
+ // constraints relative to the primary template.
+ if (ForConstraintInstantiation && !SemaRef.CodeSynthesisContexts.empty()) {
+ for (auto &CSC : llvm::reverse(SemaRef.CodeSynthesisContexts)) {
+ if (CSC.Kind != Sema::CodeSynthesisContext::SynthesisKind::
+ TypeAliasTemplateInstantiation)
+ continue;
+ auto *TATD = cast<TypeAliasTemplateDecl>(CSC.Entity),
+ *CurrentTATD = TATD;
+ FunctionDecl *LambdaCallOperator = Rec->getLambdaCallOperator();
+ // Retrieve the 'primary' template for a lambda call operator. It's
+ // unfortunate that we only have the mappings of call operators rather
+ // than lambda classes.
+ while (true) {
+ auto *FTD = dyn_cast_if_present<FunctionTemplateDecl>(
+ LambdaCallOperator->getDescribedTemplate());
+ if (FTD && FTD->getInstantiatedFromMemberTemplate()) {
+ LambdaCallOperator =
+ FTD->getInstantiatedFromMemberTemplate()->getTemplatedDecl();
+ } else if (auto *Prev = cast<CXXMethodDecl>(LambdaCallOperator)
+ ->getInstantiatedFromMemberFunction())
+ LambdaCallOperator = Prev;
+ else
+ break;
+ }
+ // Same applies for type alias Decl. We perform this to obtain the
+ // "canonical" template parameter depths.
+ while (TATD->getInstantiatedFromMemberTemplate())
+ TATD = TATD->getInstantiatedFromMemberTemplate();
+ // Tell if we're currently inside of a lambda expression that is
+ // surrounded by a using alias declaration. e.g.
+ // template <class> using type = decltype([](auto) { ^ }());
+ // By checking if:
+ // 1. The lambda expression and the using alias declaration share the
+ // same declaration context.
+ // 2. They have the same template depth.
+ // Then we assume the template arguments from the using alias
+ // declaration are essential for constraint instantiation. We have to do
+ // so since a TypeAliasTemplateDecl (or a TypeAliasDecl) is never a
+ // DeclContext, nor does it have an associated specialization Decl from
+ // which we could collect these template arguments.
+ if (cast<CXXRecordDecl>(LambdaCallOperator->getDeclContext())
+ ->getTemplateDepth() == TATD->getTemplateDepth() &&
+ getLambdaAwareParentOfDeclContext(LambdaCallOperator) ==
+ TATD->getDeclContext()) {
+ Result.addOuterTemplateArguments(CurrentTATD,
+ CSC.template_arguments(),
+ /*Final=*/false);
+ // Visit the parent of the current type alias declaration rather than
+ // the lambda thereof. We have the following case:
+ // struct S {
+ // template <class> using T = decltype([]<Concept> {} ());
+ // };
+ // void foo() {
+ // S::T var;
+ // }
+ // The instantiated lambda expression (which we're visiting at 'var')
+ // has a function DeclContext 'foo' rather than the Record DeclContext
+ // S. This seems to be an oversight that we may want to set a Sema
+ // Context from the CXXScopeSpec before substituting into T to me.
+ return Response::ChangeDecl(CurrentTATD->getDeclContext());
+ }
+ }
+ }
+ }
return Response::UseNextDecl(Rec);
}
@@ -412,7 +478,8 @@ MultiLevelTemplateArgumentList Sema::getTemplateInstantiationArgs(
R = HandleFunction(Function, Result, Pattern, RelativeToPrimary,
ForConstraintInstantiation);
} else if (const auto *Rec = dyn_cast<CXXRecordDecl>(CurDecl)) {
- R = HandleRecordDecl(Rec, Result, Context, ForConstraintInstantiation);
+ R = HandleRecordDecl(*this, Rec, Result, Context,
+ ForConstraintInstantiation);
} else if (const auto *CSD =
dyn_cast<ImplicitConceptSpecializationDecl>(CurDecl)) {
R = HandleImplicitConceptSpecializationDecl(CSD, Result);
@@ -469,6 +536,7 @@ bool Sema::CodeSynthesisContext::isInstantiationRecord() const {
case BuildingBuiltinDumpStructCall:
case LambdaExpressionSubstitution:
case BuildingDeductionGuides:
+ case TypeAliasTemplateInstantiation:
return false;
// This function should never be called when Kind's value is Memoization.
@@ -614,6 +682,15 @@ Sema::InstantiatingTemplate::InstantiatingTemplate(
PointOfInstantiation, InstantiationRange, Param, Template,
TemplateArgs) {}
+Sema::InstantiatingTemplate::InstantiatingTemplate(
+ Sema &SemaRef, SourceLocation PointOfInstantiation,
+ TypeAliasTemplateDecl *Template, ArrayRef<TemplateArgument> TemplateArgs,
+ SourceRange InstantiationRange)
+ : InstantiatingTemplate(
+ SemaRef, Sema::CodeSynthesisContext::TypeAliasTemplateInstantiation,
+ PointOfInstantiation, InstantiationRange, /*Entity=*/Template,
+ nullptr, TemplateArgs) {}
+
Sema::InstantiatingTemplate::InstantiatingTemplate(
Sema &SemaRef, SourceLocation PointOfInstantiation, TemplateDecl *Template,
NamedDecl *Param, ArrayRef<TemplateArgument> TemplateArgs,
@@ -1131,6 +1208,8 @@ void Sema::PrintInstantiationStack() {
Diags.Report(Active->PointOfInstantiation,
diag::note_building_deduction_guide_here);
break;
+ case CodeSynthesisContext::TypeAliasTemplateInstantiation:
+ break;
}
}
}
@@ -1208,6 +1287,7 @@ std::optional<TemplateDeductionInfo *> Sema::isSFINAEContext() const {
break;
case CodeSynthesisContext::Memoization:
+ case CodeSynthesisContext::TypeAliasTemplateInstantiation:
break;
}
@@ -1479,6 +1559,18 @@ namespace {
SubstTemplateTypeParmPackTypeLoc TL,
bool SuppressObjCLifetime);
+ CXXRecordDecl::LambdaDependencyKind
+ ComputeLambdaDependency(LambdaScopeInfo *LSI) {
+ auto &CCS = SemaRef.CodeSynthesisContexts.back();
+ if (CCS.Kind ==
+ Sema::CodeSynthesisContext::TypeAliasTemplateInstantiation) {
+ unsigned TypeAliasDeclDepth = CCS.Entity->getTemplateDepth();
+ if (TypeAliasDeclDepth >= TemplateArgs.getNumSubstitutedLevels())
+ return CXXRecordDecl::LambdaDependencyKind::LDK_AlwaysDependent;
+ }
+ return inherited::ComputeLambdaDependency(LSI);
+ }
+
ExprResult TransformLambdaExpr(LambdaExpr *E) {
LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true);
Sema::ConstraintEvalRAII<TemplateInstantiator> RAII(*this);
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 9c696e072ba4a7..2d8675690972ff 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1083,6 +1083,11 @@ TemplateDeclInstantiator::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
return nullptr;
TypeAliasDecl *Pattern = D->getTemplatedDecl();
+ Sema::InstantiatingTemplate InstTemplate(
+ SemaRef, Pattern->getTypeSourceInfo()->getTypeLoc().getBeginLoc(), D,
+ D->getTemplateDepth() >= TemplateArgs.getNumSubstitutedLevels()
+ ? ArrayRef<TemplateArgument>()
+ : TemplateArgs.getInnermost());
TypeAliasTemplateDecl *PrevAliasTemplate = nullptr;
if (getPreviousDeclForInstantiation<TypedefNameDecl>(Pattern)) {
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index a32a585531873a..41633646863536 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -767,6 +767,12 @@ class TreeTransform {
/// the body.
StmtResult SkipLambdaBody(LambdaExpr *E, Stmt *Body);
+ CXXRecordDecl::LambdaDependencyKind
+ ComputeLambdaDependency(LambdaScopeInfo *LSI) {
+ return static_cast<CXXRecordDecl::LambdaDependencyKind>(
+ LSI->Lambda->getLambdaDependencyKind());
+ }
+
QualType TransformReferenceType(TypeLocBuilder &TLB, ReferenceTypeLoc TL);
StmtResult TransformCompoundStmt(CompoundStmt *S, bool IsStmtExpr);
@@ -13905,6 +13911,31 @@ TreeTransform<Derived>::TransformLambdaExpr(LambdaExpr *E) {
/*IsInstantiation*/ true);
SavedContext.pop();
+ // Recompute the dependency of the lambda so that we can defer the lambda call
+ // construction until after we have sufficient template arguments. For
+ // example, template <class> struct S {
+ // template <class U>
+ // using Type = decltype([](U){}(42.0));
+ // };
+ // void foo() {
+ // using T = S<int>::Type<float>;
+ // ^~~~~~
+ // }
+ // We would end up here from instantiating the S<int> as we're ensuring the
+ // completeness. That would make us transform the lambda call expression
+ // despite the fact that we don't see the argument for U yet. We have a
+ // mechanism that circumvents the semantic checking if the CallExpr is
+ // dependent. We can harness that by recomputing the lambda dependency from
+ // the instantiation arguments. I'm putting it here rather than the above
+ // since we can see transformed lambda parameters in case that they're
+ // useful for calculation.
+ DependencyKind = getDerived().ComputeLambdaDependency(&LSICopy);
+ Class->setLambdaDependencyKind(DependencyKind);
+ // Clean up the type cache created previously. Then, we re-create a type for
+ // such Decl with the new DependencyKind.
+ Class->setTypeForDecl(nullptr);
+ getSema().Context.getTypeDeclType(Class);
+
return getSema().BuildLambdaExpr(E->getBeginLoc(), Body.get()->getEndLoc(),
&LSICopy);
}
diff --git a/clang/test/SemaTemplate/alias-template-with-lambdas.cpp b/clang/test/SemaTemplate/alias-template-with-lambdas.cpp
new file mode 100644
index 00000000000000..c3931287cb6404
--- /dev/null
+++ b/clang/test/SemaTemplate/alias-template-with-lambdas.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -std=c++2c -fsyntax-only -verify %s
+namespace lambda_calls {
+
+template <class>
+concept True = true;
+
+template <class>
+concept False = false; // #False
+
+template <class T> struct S {
+ template <class... U> using type = decltype([](U...) {}(U()...));
+ template <class U> using type2 = decltype([](auto) {}(1));
+ template <class U> using type3 = decltype([](True auto) {}(1));
+ template <class>
+ using type4 = decltype([](auto... pack) { return sizeof...(pack); }(1, 2));
+
+ template <class U> using type5 = decltype([](False auto...) {}(1)); // #Type5
+
+ template <class U>
+ using type6 = decltype([]<True> {}.template operator()<char>());
+ template <class U>
+ using type7 = decltype([]<False> {}.template operator()<char>()); // #Type7
+
+ template <class U>
+ using type8 = decltype([]() // #Type8
+ requires(sizeof(U) == 32) // #Type8-requirement
+ {}());
+
+ template <class... U>
+ using type9 = decltype([]<True>(U...) {}.template operator()<char>(U()...));
+ // https://github.com/llvm/llvm-project/issues/76674
+ template <class U>
+ using type10 = decltype([]<class V> { return V(); }.template operator()<U>());
+
+ template <class U> using type11 = decltype([] { return U{}; });
+};
+
+template <class> using Meow = decltype([]<True> {}.template operator()<int>());
+
+template <class... U>
+using MeowMeow = decltype([]<True>(U...) {}.template operator()<char>(U()...));
+
+// https://github.com/llvm/llvm-project/issues/70601
+template <class> using U = decltype([]<True> {}.template operator()<int>());
+
+U<int> foo();
+
+void bar() {
+ using T = S<int>::type<int, int, int>;
+ using T2 = S<int>::type2<int>;
+ using T3 = S<int>::type3<char>;
+ using T4 = S<int>::type4<void>;
+ using T5 = S<int>::type5<void>; // #T5
+ // expected-error@#Type5 {{no matching function for call}}
+ // expected-note@#T5 {{type alias 'type5' requested here}}
+ // expected-note@#Type5 {{constraints not satisfied [with auto:1 = <int>]}}
+ // expected-note@#Type5 {{because 'int' does not satisfy 'False'}}
+ // expected-note@#False {{because 'false' evaluated to false}}
+
+ using T6 = S<int>::type6<void>;
+ using T7 = S<int>::type7<void>; // #T7
+ // expected-error@#Type7 {{no matching member function for call}}
+ // expected-note@#T7 {{type alias 'type7' requested here}}
+ // expected-note@#Type7 {{constraints not satisfied [with $0 = char]}}
+ // expected-note@#Type7 {{because 'char' does not satisfy 'False'}}
+ // expected-note@#False {{because 'false' evaluated to false}}
+
+ using T8 = S<int>::type8<char>; // #T8
+ // expected-error@#Type8 {{no matching function for call}}
+ // expected-note@#T8 {{type alias 'type8' requested here}}
+ // expected-note@#Type8 {{constraints not satisfied}}
+ // expected-note@#Type8-requirement {{because 'sizeof(char) == 32' (1 == 32) evaluated to false}}
+
+ using T9 = S<int>::type9<long, long, char>;
+ using T10 = S<int>::type10<int>;
+ using T11 = S<int>::type11<int>;
+ int x = T11()();
+ using T12 = Meow<int>;
+ using T13 = MeowMeow<char, int, long, unsigned>;
+}
+
+} // namespace lambda_calls
|
|
Also added @shafik for some insights on tests / other existing issues. |
|
Although this patch doesn't fix #82104 yet, which is a crash caused by a mismatch in the template argument during the return type deduction, I still tried a temporary workaround (turning some switches off / on for getTemplateInstantiationArgs), and that proves we can move forward the fix with this patch. |
|
Friendly ping. I'm looking for your feedback before fixing other similar bugs, e.g. #82104. |
erichkeane
reviewed
Feb 28, 2024
There was a problem hiding this comment.
I don't have any concerns, but want @cor3ntin to take a look. He's under the weather at the moment, so he might be a few days.
cor3ntin
reviewed
Mar 5, 2024
Comment on lines
+13932
to
+13984
| DependencyKind = getDerived().ComputeLambdaDependency(&LSICopy); | ||
| Class->setLambdaDependencyKind(DependencyKind); | ||
| // Clean up the type cache created previously. Then, we re-create a type for | ||
| // such Decl with the new DependencyKind. | ||
| Class->setTypeForDecl(nullptr); | ||
| getSema().Context.getTypeDeclType(Class); | ||
|
|
There was a problem hiding this comment.
This is not great, but I am not sure how we can improve.
I think it's worth a fixme
@erichkeane
zyn0217
force-pushed
the
lambda-call-inside-of-type-aliases
branch
from
4883606 to
afd4188
Compare
Endilll
reviewed
Mar 8, 2024
|
So, I have:
In terms of the lambda dependencies, I'm not entirely sure if we can accept the current approach. In practice, it is possible to avoid such a hack at the moment, since we don't actually rely on the lambda substitution but rather the template arguments. Do you think it will be useful in the future that we need something more e.g. the substituted parameters, to compute the dependency? @cor3ntin |
|
Probably I haven't understood these code deeply and I have some confusions on this patch. I wonder has this patch solved essential issue, or it just fixed the crash. I think some |
zyn0217
force-pushed
the
lambda-call-inside-of-type-aliases
branch
from
5a8e5cb to
4fcafbe
Compare
Added. Although the patch doesn't touch that type deduction part and thus they are theoretically not affected. |
|
Friendly ping @cor3ntin and @erichkeane if you're back in the offices. |
erichkeane
reviewed
Apr 1, 2024
There was a problem hiding this comment.
1 thought, else I don't see anything. @cor3ntin is coming back in a few days, so I'd like him to do a final once over.
| // than lambda classes. | ||
| const FunctionDecl * | ||
| getPrimaryTemplateOfGenericLambda(const FunctionDecl *LambdaCallOperator) { | ||
| while (true) { |
There was a problem hiding this comment.
What cases do we have where this takes more than 1 step? I also wonder if we'd be better off finding the primary CXXRecordDecl, then picking it up from there? We could use getTemplateInstantiationPattern then getDescribedClassTemplate I think?
There was a problem hiding this comment.
(Sorry for replying this late, I just got around to this PR.)
What cases do we have where this takes more than 1 step?
The lambda itself might be defined within a template, and I presume transforming that template introduces multilevel mappings.
I also wonder if we'd be better off finding the primary
CXXRecordDecl, then picking it up from there? We could usegetTemplateInstantiationPatternthengetDescribedClassTemplateI think?
If we examine TemplateInstantiator::transformedLocalDecl, we'd see there's only handling for CXXMethodDecls of adding such mappings that we can extract from the Decls themselves. I think it is possible that we can use the primary CXXRecordDecl approach, although looking for that Decl might involve handling LocalInstantiationScopes, which IMO is not super straightforward.
cor3ntin
approved these changes
Apr 4, 2024
|
ping @cor3ntin |
|
Thank you folks for the insightful review! I'm merging this PR now. |
|
Still crash on template<auto F> constexpr auto x = F();
template<class> constexpr int a() { return 1; }
template <class>
struct A {
using Func = decltype(
[]<class T>(T) {
return x<[] constexpr { return a<T>(); }>;
// return x<[] constexpr { return b(); }>;
}.template operator()<unsigned long long>('2')
);
};
A<short>::Func y; |
That is a distinct case: Func does not form a |
Consider this one, template<auto F> constexpr auto x = F();
template<class> constexpr int a() { return 1; }
template <class>
struct A {
template<typename U>
using Func = decltype(
[]<class T>(T) {
return x<[] constexpr { return a<T>(); }>;
}.template operator()<unsigned long long>('2')
);
};
A<short>::Func<int> y;
|
|
@jcsxky At the glance of the stacktrace, I suspect we probably need some mechanism of deferral codegen while instantiating the enclosing struct. Can you please file a separate issue? |
zyn0217
added a commit
that referenced
this pull request
Aug 27, 2024
…late decl (#89934) In the last patch #82310, we used template depths to tell if such alias decls contain lambdas, which is wrong because the lambda can also appear as a part of the default argument, and that would make `getTemplateInstantiationArgs` provide extra template arguments in undesired contexts. This leads to issue #89853. Moreover, our approach for #82104 was sadly wrong. We tried to teach `DeduceReturnType` to consider alias template arguments; however, giving these arguments in the context where they should have been substituted in a `TransformCallExpr` call is never correct. This patch addresses such problems by using a `RecursiveASTVisitor` to check if the lambda is contained by an alias `Decl`, as well as twiddling the lambda dependencies - we should also build a dependent lambda expression if the surrounding alias template arguments were dependent. Fixes #89853 Fixes #102760 Fixes #105885
llvmbot
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 27, 2024
…late decl (llvm#89934) In the last patch llvm#82310, we used template depths to tell if such alias decls contain lambdas, which is wrong because the lambda can also appear as a part of the default argument, and that would make `getTemplateInstantiationArgs` provide extra template arguments in undesired contexts. This leads to issue llvm#89853. Moreover, our approach for llvm#82104 was sadly wrong. We tried to teach `DeduceReturnType` to consider alias template arguments; however, giving these arguments in the context where they should have been substituted in a `TransformCallExpr` call is never correct. This patch addresses such problems by using a `RecursiveASTVisitor` to check if the lambda is contained by an alias `Decl`, as well as twiddling the lambda dependencies - we should also build a dependent lambda expression if the surrounding alias template arguments were dependent. Fixes llvm#89853 Fixes llvm#102760 Fixes llvm#105885 (cherry picked from commit b412ec5)
tru
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Sep 1, 2024
…late decl (llvm#89934) In the last patch llvm#82310, we used template depths to tell if such alias decls contain lambdas, which is wrong because the lambda can also appear as a part of the default argument, and that would make `getTemplateInstantiationArgs` provide extra template arguments in undesired contexts. This leads to issue llvm#89853. Moreover, our approach for llvm#82104 was sadly wrong. We tried to teach `DeduceReturnType` to consider alias template arguments; however, giving these arguments in the context where they should have been substituted in a `TransformCallExpr` call is never correct. This patch addresses such problems by using a `RecursiveASTVisitor` to check if the lambda is contained by an alias `Decl`, as well as twiddling the lambda dependencies - we should also build a dependent lambda expression if the surrounding alias template arguments were dependent. Fixes llvm#89853 Fixes llvm#102760 Fixes llvm#105885 (cherry picked from commit b412ec5)
|
This crash looks linked to this PR: #136432 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch attempts to fix the lambda call expression inside of a type alias declaration from two aspects:
It is unfortunate that a
TypeAliasTemplateDecl(or aTypeAliasDecl) is never aDeclContext, nor does it have an associated specialization Decl from which we could collect these template arguments. Thus, I added a new CodeSynthesisContext to record template arguments for alias declarations.Fixes #70601
Fixes #76674
Fixes #79555
Fixes #81145
Fixes #82104
Note that this doesn't involve the fix for #28461. That seems different, and I'd like to leave it as a follow-up.