From 5901d82ea0543074853b963f7dc9106a6fe3bcee Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Wed, 11 Sep 2024 11:33:45 +0000 Subject: [PATCH 1/7] [clang] Do not expand pack while retaining expansion --- clang/lib/Sema/TreeTransform.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 0daf620b4123e..a40673b04764d 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -4361,7 +4361,7 @@ bool TreeTransform::TransformExprs(Expr *const *Inputs, // forgetting the partially-substituted parameter pack. if (RetainExpansion) { ForgetPartiallySubstitutedPackRAII Forget(getDerived()); - + Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1); ExprResult Out = getDerived().TransformExpr(Pattern); if (Out.isInvalid()) return true; From 97fbf34c3edd09348fb48b4dc66f1d854516e8ef Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Wed, 11 Sep 2024 11:59:58 +0000 Subject: [PATCH 2/7] Add comment --- clang/lib/Sema/TreeTransform.h | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index a40673b04764d..0de43d2127b12 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -4361,6 +4361,7 @@ bool TreeTransform::TransformExprs(Expr *const *Inputs, // forgetting the partially-substituted parameter pack. if (RetainExpansion) { ForgetPartiallySubstitutedPackRAII Forget(getDerived()); + // Simple transform producing another pack expansion. Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1); ExprResult Out = getDerived().TransformExpr(Pattern); if (Out.isInvalid()) From ab756525ab4a5f71ee5d3703238a1f9188a376e1 Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Wed, 11 Sep 2024 15:45:40 +0000 Subject: [PATCH 3/7] Add tests --- clang/test/SemaCXX/GH107560.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 clang/test/SemaCXX/GH107560.cpp diff --git a/clang/test/SemaCXX/GH107560.cpp b/clang/test/SemaCXX/GH107560.cpp new file mode 100644 index 0000000000000..faaa7004e06cf --- /dev/null +++ b/clang/test/SemaCXX/GH107560.cpp @@ -0,0 +1,14 @@ +// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s +// expected-no-diagnostics + +int bar(...); + +template struct Int {}; + +template +consteval auto foo(T... x) -> decltype(bar(T(x)...)) { return 10; } + +template +constexpr auto baz(Int(T())>... x) -> int { return 1; } + +static_assert(baz, Int<2>, Int<3>>(Int<10>(), Int<10>(), Int<10>()) == 1); From 600a9259bf087c5f3b6e881211da69013aca018e Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Wed, 11 Sep 2024 16:27:49 +0000 Subject: [PATCH 4/7] Move ArgumentPackSubstitutionIndexRAII to ForgetPartiallySubstitutedPackRAII --- clang/lib/Sema/TreeTransform.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 0de43d2127b12..1538f87ac2735 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -113,9 +113,11 @@ class TreeTransform { class ForgetPartiallySubstitutedPackRAII { Derived &Self; TemplateArgument Old; + Sema::ArgumentPackSubstitutionIndexRAII ResetPackSubstIndex; public: - ForgetPartiallySubstitutedPackRAII(Derived &Self) : Self(Self) { + ForgetPartiallySubstitutedPackRAII(Derived &Self) + : Self(Self), ResetPackSubstIndex(Self.getSema(), -1) { Old = Self.ForgetPartiallySubstitutedPack(); } @@ -4361,8 +4363,6 @@ bool TreeTransform::TransformExprs(Expr *const *Inputs, // forgetting the partially-substituted parameter pack. if (RetainExpansion) { ForgetPartiallySubstitutedPackRAII Forget(getDerived()); - // Simple transform producing another pack expansion. - Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(getSema(), -1); ExprResult Out = getDerived().TransformExpr(Pattern); if (Out.isInvalid()) return true; From 6cd618e295e984f2dd1924407fe64553549e6fcb Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Wed, 11 Sep 2024 16:31:18 +0000 Subject: [PATCH 5/7] remove space --- clang/lib/Sema/TreeTransform.h | 1 + 1 file changed, 1 insertion(+) diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 1538f87ac2735..aa3e2308828be 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -4363,6 +4363,7 @@ bool TreeTransform::TransformExprs(Expr *const *Inputs, // forgetting the partially-substituted parameter pack. if (RetainExpansion) { ForgetPartiallySubstitutedPackRAII Forget(getDerived()); + ExprResult Out = getDerived().TransformExpr(Pattern); if (Out.isInvalid()) return true; From 98bf2f3d7d7c2746b6d2e5f7921b6947cd598d3b Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Thu, 12 Sep 2024 09:19:43 +0000 Subject: [PATCH 6/7] move test to existing file; add comment; --- clang/lib/Sema/TreeTransform.h | 2 ++ clang/test/SemaCXX/GH107560.cpp | 14 -------------- clang/test/SemaTemplate/pack-deduction.cpp | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 14 deletions(-) delete mode 100644 clang/test/SemaCXX/GH107560.cpp diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index aa3e2308828be..1ca5f58c87458 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -113,6 +113,8 @@ class TreeTransform { class ForgetPartiallySubstitutedPackRAII { Derived &Self; TemplateArgument Old; + // Set the pack expansion index to -1 to avoid pack substitution and + // indicate that parameter packs should be instantiated as themselves. Sema::ArgumentPackSubstitutionIndexRAII ResetPackSubstIndex; public: diff --git a/clang/test/SemaCXX/GH107560.cpp b/clang/test/SemaCXX/GH107560.cpp deleted file mode 100644 index faaa7004e06cf..0000000000000 --- a/clang/test/SemaCXX/GH107560.cpp +++ /dev/null @@ -1,14 +0,0 @@ -// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s -// expected-no-diagnostics - -int bar(...); - -template struct Int {}; - -template -consteval auto foo(T... x) -> decltype(bar(T(x)...)) { return 10; } - -template -constexpr auto baz(Int(T())>... x) -> int { return 1; } - -static_assert(baz, Int<2>, Int<3>>(Int<10>(), Int<10>(), Int<10>()) == 1); diff --git a/clang/test/SemaTemplate/pack-deduction.cpp b/clang/test/SemaTemplate/pack-deduction.cpp index e42709820e9cf..28fb127a38644 100644 --- a/clang/test/SemaTemplate/pack-deduction.cpp +++ b/clang/test/SemaTemplate/pack-deduction.cpp @@ -185,3 +185,17 @@ void Run() { Outer::Inner<0>().Test(1,1); } } + +namespace GH107560 { +int bar(...); + +template struct Int {}; + +template +constexpr auto foo(T... x) -> decltype(bar(T(x)...)) { return 10; } + +template +constexpr auto baz(Int(T())>... x) -> int { return 1; } + +static_assert(baz, Int<2>, Int<3>>(Int<10>(), Int<10>(), Int<10>()) == 1, ""); +} From 12a03c4fdceeccfb2e8f2ef448a38ba946faf7fc Mon Sep 17 00:00:00 2001 From: Utkarsh Saxena Date: Thu, 12 Sep 2024 11:04:40 +0000 Subject: [PATCH 7/7] release notes --- clang/docs/ReleaseNotes.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 59ccdf1e15cd8..af541fadbfb14 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -380,6 +380,8 @@ Bug Fixes to C++ Support - Fixed a bug where defaulted comparison operators would remove ``const`` from base classes. (#GH102588) - Fix a crash when using ``source_location`` in the trailing return type of a lambda expression. (#GH67134) - A follow-up fix was added for (#GH61460), as the previous fix was not entirely correct. (#GH86361) +- Fixed a crash when clang tries to subtitute parameter pack while retaining the parameter + pack. #GH63819, #GH107560 Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^