[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded
cor3ntin added a comment. @MitalAshok Are you still working on this? Do you need further help on this patch? You have commit access, correct? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154716/new/ https://reviews.llvm.org/D154716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154716/new/ https://reviews.llvm.org/D154716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded
MitalAshok marked an inline comment as done.
MitalAshok added inline comments.
Comment at: clang/lib/Sema/TreeTransform.h:13323
// If this is an init-capture pack, consider expanding the pack now.
if (OldVD->isParameterPack()) {
shafik wrote:
> Based on the changes is this comment still accurate?
Yes, the code was just changed so it's more clear that the result either has
`N` distinct arguments for a pack of size `N` or just 1 argument with a
(possible dependent) pack type
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154716/new/
https://reviews.llvm.org/D154716
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded
MitalAshok updated this revision to Diff 543183.
MitalAshok edited the summary of this revision.
MitalAshok added a comment.
Added entry to changelog and the test from the bug report
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154716/new/
https://reviews.llvm.org/D154716
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/lambda-pack-expansion.cpp
Index: clang/test/SemaCXX/lambda-pack-expansion.cpp
===
--- clang/test/SemaCXX/lambda-pack-expansion.cpp
+++ clang/test/SemaCXX/lambda-pack-expansion.cpp
@@ -21,3 +21,30 @@
take_by_ref(x);
}
}
+
+namespace GH63677 {
+
+template
+void f() {
+ []() -> void {
+[...us = Ts{}]{
+ (Ts(us), ...);
+};
+ }.template operator()();
+}
+
+template void f();
+
+template
+inline constexpr auto fun =
+ [](Ts... ts) {
+return [... us = (Ts&&) ts](Fun&& fn) mutable {
+ return static_cast(fn)(static_cast(us)...);
+};
+ };
+
+void f() {
+ [[maybe_unused]] auto s = fun(1, 2, 3, 4);
+}
+
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13340,14 +13340,14 @@
OldVD->getInit()->getSourceRange(), Unexpanded, Expand,
RetainExpansion, NumExpansions))
return ExprError();
+ assert(!RetainExpansion && "Should not need to retain expansion after a "
+ "capture since it cannot be extended");
if (Expand) {
for (unsigned I = 0; I != *NumExpansions; ++I) {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
SubstInitCapture(SourceLocation(), std::nullopt);
}
- }
- if (!Expand || RetainExpansion) {
-ForgetPartiallySubstitutedPackRAII Forget(getDerived());
+ } else {
SubstInitCapture(ExpansionTL.getEllipsisLoc(), NumExpansions);
Result.EllipsisLoc = ExpansionTL.getEllipsisLoc();
}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1270,7 +1270,8 @@
}
void transformedLocalDecl(Decl *Old, ArrayRef NewDecls) {
- if (Old->isParameterPack()) {
+ if (Old->isParameterPack() &&
+ (NewDecls.size() != 1 || !NewDecls.front()->isParameterPack())) {
SemaRef.CurrentInstantiationScope->MakeInstantiatedLocalArgPack(Old);
for (auto *New : NewDecls)
SemaRef.CurrentInstantiationScope->InstantiatedLocalPackArg(
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -782,6 +782,8 @@
(`#63903 `_)
- Fix constraint checking of non-generic lambdas.
(`#63181 `_)
+- Fix init-capture packs having a size of one before being instantiated
+ (`#63677 `_)
Bug Fixes to AST Handling
^
Index: clang/test/SemaCXX/lambda-pack-expansion.cpp
===
--- clang/test/SemaCXX/lambda-pack-expansion.cpp
+++ clang/test/SemaCXX/lambda-pack-expansion.cpp
@@ -21,3 +21,30 @@
take_by_ref(x);
}
}
+
+namespace GH63677 {
+
+template
+void f() {
+ []() -> void {
+[...us = Ts{}]{
+ (Ts(us), ...);
+};
+ }.template operator()();
+}
+
+template void f();
+
+template
+inline constexpr auto fun =
+ [](Ts... ts) {
+return [... us = (Ts&&) ts](Fun&& fn) mutable {
+ return static_cast(fn)(static_cast(us)...);
+};
+ };
+
+void f() {
+ [[maybe_unused]] auto s = fun(1, 2, 3, 4);
+}
+
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13340,14 +13340,14 @@
OldVD->getInit()->getSourceRange(), Unexpanded, Expand,
RetainExpansion, NumExpansions))
return ExprError();
+ assert(!RetainExpansion && "Should not need to retain expansion after a "
+ "capture since it cannot be extended");
if (Expand) {
for (unsigned I = 0; I != *NumExpansions; ++I) {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
SubstInitCapture(SourceLocation(), std::nullopt);
}
- }
- if (!Expand || RetainExpansion) {
-ForgetPartiallySubstitutedPackRAII Forget(getDerived());
+ } else {
SubstInitCapture(ExpansionTL.getEllipsisLoc(),
[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded
shafik added inline comments.
Comment at: clang/lib/Sema/TreeTransform.h:13323
// If this is an init-capture pack, consider expanding the pack now.
if (OldVD->isParameterPack()) {
Based on the changes is this comment still accurate?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154716/new/
https://reviews.llvm.org/D154716
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded
shafik added inline comments. Comment at: clang/test/SemaCXX/lambda-pack-expansion.cpp:36 + +template void f(); + Can we also have an example similar to the bug report where we have multiple arguments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154716/new/ https://reviews.llvm.org/D154716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded
cor3ntin added a comment. This looks reasonable to me. But can you add an entry into clang/docs/ReleaseNotes.rst? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154716/new/ https://reviews.llvm.org/D154716 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded
MitalAshok created this revision.
Herald added a project: All.
MitalAshok edited the summary of this revision.
MitalAshok added reviewers: rsmith, cor3ntin.
MitalAshok published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
When instantiating the closure type of a lambda in a template, sometimes a
capture which is a pack is not expanded. When that happens, it is replaced with
a pack of size 1 containing that pack. Before it is replaced by another
instantiation where the pack is expanded, the size is reported to be 1 instead
of unknown.
This checks if this is happening (i.e., trying to replace a pack declaration
with a single declaration that is itself a pack), and not expanding it out in
that case.
Fixes https://github.com/llvm/llvm-project/issues/63677
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154716
Files:
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/lambda-pack-expansion.cpp
Index: clang/test/SemaCXX/lambda-pack-expansion.cpp
===
--- clang/test/SemaCXX/lambda-pack-expansion.cpp
+++ clang/test/SemaCXX/lambda-pack-expansion.cpp
@@ -21,3 +21,18 @@
take_by_ref(x);
}
}
+
+namespace GH63677 {
+
+template
+void f() {
+[]() -> void {
+[...us = Ts{}]{
+(Ts(us), ...);
+};
+}.template operator()();
+}
+
+template void f();
+
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13340,14 +13340,14 @@
OldVD->getInit()->getSourceRange(), Unexpanded, Expand,
RetainExpansion, NumExpansions))
return ExprError();
+ assert(!RetainExpansion && "Should not need to retain expansion after a "
+ "capture since it cannot be extended");
if (Expand) {
for (unsigned I = 0; I != *NumExpansions; ++I) {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
SubstInitCapture(SourceLocation(), std::nullopt);
}
- }
- if (!Expand || RetainExpansion) {
-ForgetPartiallySubstitutedPackRAII Forget(getDerived());
+ } else {
SubstInitCapture(ExpansionTL.getEllipsisLoc(), NumExpansions);
Result.EllipsisLoc = ExpansionTL.getEllipsisLoc();
}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1270,7 +1270,8 @@
}
void transformedLocalDecl(Decl *Old, ArrayRef NewDecls) {
- if (Old->isParameterPack()) {
+ if (Old->isParameterPack() &&
+ (NewDecls.size() != 1 || !NewDecls.front()->isParameterPack())) {
SemaRef.CurrentInstantiationScope->MakeInstantiatedLocalArgPack(Old);
for (auto *New : NewDecls)
SemaRef.CurrentInstantiationScope->InstantiatedLocalPackArg(
Index: clang/test/SemaCXX/lambda-pack-expansion.cpp
===
--- clang/test/SemaCXX/lambda-pack-expansion.cpp
+++ clang/test/SemaCXX/lambda-pack-expansion.cpp
@@ -21,3 +21,18 @@
take_by_ref(x);
}
}
+
+namespace GH63677 {
+
+template
+void f() {
+[]() -> void {
+[...us = Ts{}]{
+(Ts(us), ...);
+};
+}.template operator()();
+}
+
+template void f();
+
+}
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -13340,14 +13340,14 @@
OldVD->getInit()->getSourceRange(), Unexpanded, Expand,
RetainExpansion, NumExpansions))
return ExprError();
+ assert(!RetainExpansion && "Should not need to retain expansion after a "
+ "capture since it cannot be extended");
if (Expand) {
for (unsigned I = 0; I != *NumExpansions; ++I) {
Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), I);
SubstInitCapture(SourceLocation(), std::nullopt);
}
- }
- if (!Expand || RetainExpansion) {
-ForgetPartiallySubstitutedPackRAII Forget(getDerived());
+ } else {
SubstInitCapture(ExpansionTL.getEllipsisLoc(), NumExpansions);
Result.EllipsisLoc = ExpansionTL.getEllipsisLoc();
}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1270,7 +1270,8 @@
}
void transformedLocalDecl(Decl *Old, ArrayRef NewDecls) {
- if (Old->isParameterPack()) {
+ if (Old->isParameterPack() &&
+
