[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -951,28 +959,130 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator 
&D,
   return New;
 }
 
+namespace {
+// CheckBindingsCount
+//  - Checks the arity of the structured bindings
+//  - Creates the resolved pack expr if there is
+//one
+bool CheckBindingsCount(Sema &S, DecompositionDecl *DD, QualType DecompType,
+ArrayRef Bindings,
+unsigned MemberCount) {
+  auto BindingWithPackItr =
+  std::find_if(Bindings.begin(), Bindings.end(),
+   [](BindingDecl *D) -> bool { return D->isParameterPack(); 
});
+  bool HasPack = BindingWithPackItr != Bindings.end();
+  bool IsValid;
+  if (!HasPack) {
+IsValid = Bindings.size() == MemberCount;
+  } else {
+// there may not be more members than non-pack bindings
+IsValid = MemberCount >= Bindings.size() - 1;
+  }
+
+  if (IsValid && HasPack) {
+TemplateTypeParmDecl *DummyTemplateParam = TemplateTypeParmDecl::Create(
+S.Context, S.Context.getTranslationUnitDecl(),
+/*KeyLoc*/ SourceLocation(), /*NameLoc*/ SourceLocation(),
+/*TemplateDepth*/ 0, /*AutoParameterPosition*/ 0,
+/*Identifier*/ nullptr, false, /*IsParameterPack*/ true);
+
+// create the pack expr and assign it to the binding
+unsigned PackSize = MemberCount - Bindings.size() + 1;
+QualType PackType = S.Context.getPackExpansionType(
+QualType(DummyTemplateParam->getTypeForDecl(), 0), PackSize);
+(*BindingWithPackItr)
+->setBinding(PackType,
+ ResolvedUnexpandedPackExpr::Create(
+ S.Context, DD->getBeginLoc(), DecompType, PackSize));
+  }
+
+  if (IsValid)
+return false;
+
+  S.Diag(DD->getLocation(), diag::err_decomp_decl_wrong_number_bindings)
+  << DecompType << (unsigned)Bindings.size() << MemberCount << MemberCount
+  << (MemberCount < Bindings.size());
+  return true;
+}
+
+// BindingInitWalker
+//  - This implements a forward iterating flattened view
+//of structured bindings that may have a nested pack.
+//It allows the user to set the init expr for either the
+//BindingDecl or its ResolvedUnexpandedPackExpr

ricejasonf wrote:

By contiguous, I mean that we would not nest the BindingDecls. The trade off is 
the extra references needed would bloat the size of DecompositionDecl. Perhaps 
that is a concern if there are a ton of them in a program which is why I 
abandoned the idea. The problem here is that the bindings are created before 
the init expression so we do not know how many bindings there will be up front 
in the case of packs. Hence the weird BindingInitWalker stuff as well as the 
DecompositionDecl::Visit* functions that I added. If you want me to remove 
those, it will require deferring the allocation of the BindingDecls.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -805,7 +805,6 @@ Sema::InstantiatingTemplate::InstantiatingTemplate(
   SemaRef, CodeSynthesisContext::BuildingDeductionGuides,
   PointOfInstantiation, InstantiationRange, Entity) {}
 
-

ricejasonf wrote:

Sorry, `git clang-format` keeps modifying this.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -15991,6 +15991,13 @@ 
TreeTransform::TransformFunctionParmPackExpr(FunctionParmPackExpr *E) {
   return E;
 }
 
+template 
+ExprResult TreeTransform::TransformResolvedUnexpandedPackExpr(
+ResolvedUnexpandedPackExpr *E) {
+  // Default behavior is to do nothing with this transformation.

ricejasonf wrote:

I was about to do this, but I see I followed the convention of the other 
ParmPackExpr kinds above it. Are you sure you want me to move template 
instantiation stuff into TreeTransform?

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -951,28 +959,130 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator 
&D,
   return New;
 }
 
+namespace {
+// CheckBindingsCount
+//  - Checks the arity of the structured bindings
+//  - Creates the resolved pack expr if there is
+//one
+bool CheckBindingsCount(Sema &S, DecompositionDecl *DD, QualType DecompType,
+ArrayRef Bindings,
+unsigned MemberCount) {
+  auto BindingWithPackItr =
+  std::find_if(Bindings.begin(), Bindings.end(),
+   [](BindingDecl *D) -> bool { return D->isParameterPack(); 
});
+  bool HasPack = BindingWithPackItr != Bindings.end();
+  bool IsValid;
+  if (!HasPack) {
+IsValid = Bindings.size() == MemberCount;
+  } else {
+// there may not be more members than non-pack bindings
+IsValid = MemberCount >= Bindings.size() - 1;
+  }
+
+  if (IsValid && HasPack) {
+TemplateTypeParmDecl *DummyTemplateParam = TemplateTypeParmDecl::Create(
+S.Context, S.Context.getTranslationUnitDecl(),
+/*KeyLoc*/ SourceLocation(), /*NameLoc*/ SourceLocation(),
+/*TemplateDepth*/ 0, /*AutoParameterPosition*/ 0,
+/*Identifier*/ nullptr, false, /*IsParameterPack*/ true);
+
+// create the pack expr and assign it to the binding
+unsigned PackSize = MemberCount - Bindings.size() + 1;
+QualType PackType = S.Context.getPackExpansionType(
+QualType(DummyTemplateParam->getTypeForDecl(), 0), PackSize);
+(*BindingWithPackItr)
+->setBinding(PackType,
+ ResolvedUnexpandedPackExpr::Create(
+ S.Context, DD->getBeginLoc(), DecompType, PackSize));
+  }
+
+  if (IsValid)
+return false;
+
+  S.Diag(DD->getLocation(), diag::err_decomp_decl_wrong_number_bindings)
+  << DecompType << (unsigned)Bindings.size() << MemberCount << MemberCount
+  << (MemberCount < Bindings.size());
+  return true;
+}
+
+// BindingInitWalker
+//  - This implements a forward iterating flattened view
+//of structured bindings that may have a nested pack.
+//It allows the user to set the init expr for either the
+//BindingDecl or its ResolvedUnexpandedPackExpr

ricejasonf wrote:

Do you mean that we should not have BindingDecls as TrailingObjects for the 
DecompositionDecl? I started doing something like this and it did mean that we 
would have a contiguous array of bindings. We would have to store a pointer to 
the BindingDecl that represents a pack to transform it. (I kind of started to 
do this and then abandoned it.)

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -951,28 +959,130 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator 
&D,
   return New;
 }
 
+namespace {
+// CheckBindingsCount
+//  - Checks the arity of the structured bindings
+//  - Creates the resolved pack expr if there is
+//one
+bool CheckBindingsCount(Sema &S, DecompositionDecl *DD, QualType DecompType,
+ArrayRef Bindings,
+unsigned MemberCount) {
+  auto BindingWithPackItr =
+  std::find_if(Bindings.begin(), Bindings.end(),
+   [](BindingDecl *D) -> bool { return D->isParameterPack(); 
});
+  bool HasPack = BindingWithPackItr != Bindings.end();
+  bool IsValid;
+  if (!HasPack) {
+IsValid = Bindings.size() == MemberCount;
+  } else {
+// there may not be more members than non-pack bindings
+IsValid = MemberCount >= Bindings.size() - 1;
+  }
+
+  if (IsValid && HasPack) {
+TemplateTypeParmDecl *DummyTemplateParam = TemplateTypeParmDecl::Create(
+S.Context, S.Context.getTranslationUnitDecl(),
+/*KeyLoc*/ SourceLocation(), /*NameLoc*/ SourceLocation(),
+/*TemplateDepth*/ 0, /*AutoParameterPosition*/ 0,
+/*Identifier*/ nullptr, false, /*IsParameterPack*/ true);

ricejasonf wrote:

We are sort of creating an implicit template and need a dummy template 
parameter. It is really only used to create the pack expansion type. This is to 
create an Expr that the BindingDecl is "aliasing".

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/121417

>From 3c81c5bd989f26331917f1401becc1cfa7f4a454 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Thu, 22 Jul 2021 16:46:33 -0700
Subject: [PATCH 1/3] [Clang][P1061] stuctured binding packs

---
 clang/include/clang/AST/Decl.h|   8 +-
 clang/include/clang/AST/DeclCXX.h |  22 +-
 clang/include/clang/AST/ExprCXX.h |  48 
 clang/include/clang/AST/RecursiveASTVisitor.h |   1 +
 .../clang/Basic/DiagnosticParseKinds.td   |   5 +
 .../clang/Basic/DiagnosticSemaKinds.td|   3 +
 clang/include/clang/Basic/StmtNodes.td|   1 +
 clang/include/clang/Sema/DeclSpec.h   |   1 +
 clang/include/clang/Sema/Sema.h   |   4 +-
 .../include/clang/Serialization/ASTBitCodes.h |   1 +
 clang/lib/AST/ASTContext.cpp  |  14 +-
 clang/lib/AST/ASTImporter.cpp |   2 +-
 clang/lib/AST/Decl.cpp|  11 +-
 clang/lib/AST/DeclBase.cpp|   6 +-
 clang/lib/AST/DeclCXX.cpp |  60 -
 clang/lib/AST/Expr.cpp|   5 +
 clang/lib/AST/ExprCXX.cpp |  48 
 clang/lib/AST/ExprClassification.cpp  |   7 +
 clang/lib/AST/ExprConstant.cpp|   5 +-
 clang/lib/AST/ItaniumMangle.cpp   |   2 +-
 clang/lib/AST/StmtPrinter.cpp |  11 +
 clang/lib/AST/StmtProfile.cpp |   4 +
 clang/lib/CodeGen/CGDebugInfo.cpp |   4 +-
 clang/lib/CodeGen/CGDecl.cpp  |   5 +-
 clang/lib/CodeGen/CodeGenModule.cpp   |   4 +-
 clang/lib/Parse/ParseDecl.cpp |  28 ++-
 clang/lib/Sema/SemaDecl.cpp   |  10 +
 clang/lib/Sema/SemaDeclCXX.cpp| 209 ++
 clang/lib/Sema/SemaExceptionSpec.cpp  |   7 +-
 clang/lib/Sema/SemaLambda.cpp |   1 +
 clang/lib/Sema/SemaStmt.cpp   |  14 +-
 clang/lib/Sema/SemaTemplate.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  40 +++-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  32 ++-
 clang/lib/Sema/SemaTemplateVariadic.cpp   |  73 +-
 clang/lib/Sema/TreeTransform.h|   7 +
 clang/lib/Serialization/ASTReaderStmt.cpp |  11 +
 clang/lib/Serialization/ASTWriter.cpp |   1 +
 clang/lib/Serialization/ASTWriterStmt.cpp |  11 +
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp  |   1 +
 clang/test/Parser/cxx2c-binding-pack.cpp  |   7 +
 .../cxx2c-binding-pack-nontemplate.cpp|  12 +
 clang/test/SemaCXX/cxx2c-binding-pack.cpp |  82 +++
 clang/test/SemaCXX/typo-correction-crash.cpp  |   3 +-
 clang/tools/libclang/CXCursor.cpp |   1 +
 45 files changed, 728 insertions(+), 111 deletions(-)
 create mode 100644 clang/test/Parser/cxx2c-binding-pack.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack.cpp

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..bdf6c81732d0bc 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -698,6 +698,10 @@ class ValueDecl : public NamedDecl {
 return const_cast(this)->getPotentiallyDecomposedVarDecl();
   }
 
+  /// Determine whether this value is actually a function parameter pack,
+  /// init-capture pack, or structured binding pack
+  bool isParameterPack() const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
@@ -1527,10 +1531,6 @@ class VarDecl : public DeclaratorDecl, public 
Redeclarable {
 NonParmVarDeclBits.IsInitCapture = IC;
   }
 
-  /// Determine whether this variable is actually a function parameter pack or
-  /// init-capture pack.
-  bool isParameterPack() const;
-
   /// Whether this local extern variable declaration's previous declaration
   /// was declared in the same block scope. Only correct in C++.
   bool isPreviousDeclInSameBlockScope() const {
diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index c232556edeff70..12002db17fb3ad 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4131,8 +4131,9 @@ class BindingDecl : public ValueDecl {
   /// binding).
   Expr *Binding = nullptr;
 
-  BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id)
-  : ValueDecl(Decl::Binding, DC, IdLoc, Id, QualType()) {}
+  BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id,
+  QualType T)
+  : ValueDecl(Decl::Binding, DC, IdLoc, Id, T) {}
 
   void anchor() override;
 
@@ -4140,7 +4141,8 @@ class BindingDecl : public ValueDecl {
   friend class ASTDeclReader;
 
   static BindingDecl *Create(ASTContext

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -928,9 +928,10 @@ static TemplateArgumentLoc translateTemplateArgument(Sema 
&SemaRef,
 
 void Sema::translateTemplateArguments(const ASTTemplateArgsPtr &TemplateArgsIn,
   TemplateArgumentListInfo &TemplateArgs) {
- for (unsigned I = 0, Last = TemplateArgsIn.size(); I != Last; ++I)
-   TemplateArgs.addArgument(translateTemplateArgument(*this,
-  TemplateArgsIn[I]));
+  for (unsigned I = 0, Last = TemplateArgsIn.size(); I != Last; ++I) {
+TemplateArgs.addArgument(
+translateTemplateArgument(*this, TemplateArgsIn[I]));
+  }

ricejasonf wrote:

I removed the braces, but clang-format keeps picking this up because there is 
only one space of indentation. (I am guessing that would be an issue with CI.)

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -755,7 +755,7 @@ bool Sema::checkMustTailAttr(const Stmt *St, const Attr 
&MTA) {
 return true;
   };
 
-  const auto *CallerDecl = dyn_cast(CurContext);
+  const auto *CallerDecl = getCurFunctionDecl();

ricejasonf wrote:

Stuff like this is from the implicit template region stuffs. There is another 
one like that that will likely fix that attr-tail.m test that was failing.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/121417

>From 3c81c5bd989f26331917f1401becc1cfa7f4a454 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Thu, 22 Jul 2021 16:46:33 -0700
Subject: [PATCH 1/4] [Clang][P1061] stuctured binding packs

---
 clang/include/clang/AST/Decl.h|   8 +-
 clang/include/clang/AST/DeclCXX.h |  22 +-
 clang/include/clang/AST/ExprCXX.h |  48 
 clang/include/clang/AST/RecursiveASTVisitor.h |   1 +
 .../clang/Basic/DiagnosticParseKinds.td   |   5 +
 .../clang/Basic/DiagnosticSemaKinds.td|   3 +
 clang/include/clang/Basic/StmtNodes.td|   1 +
 clang/include/clang/Sema/DeclSpec.h   |   1 +
 clang/include/clang/Sema/Sema.h   |   4 +-
 .../include/clang/Serialization/ASTBitCodes.h |   1 +
 clang/lib/AST/ASTContext.cpp  |  14 +-
 clang/lib/AST/ASTImporter.cpp |   2 +-
 clang/lib/AST/Decl.cpp|  11 +-
 clang/lib/AST/DeclBase.cpp|   6 +-
 clang/lib/AST/DeclCXX.cpp |  60 -
 clang/lib/AST/Expr.cpp|   5 +
 clang/lib/AST/ExprCXX.cpp |  48 
 clang/lib/AST/ExprClassification.cpp  |   7 +
 clang/lib/AST/ExprConstant.cpp|   5 +-
 clang/lib/AST/ItaniumMangle.cpp   |   2 +-
 clang/lib/AST/StmtPrinter.cpp |  11 +
 clang/lib/AST/StmtProfile.cpp |   4 +
 clang/lib/CodeGen/CGDebugInfo.cpp |   4 +-
 clang/lib/CodeGen/CGDecl.cpp  |   5 +-
 clang/lib/CodeGen/CodeGenModule.cpp   |   4 +-
 clang/lib/Parse/ParseDecl.cpp |  28 ++-
 clang/lib/Sema/SemaDecl.cpp   |  10 +
 clang/lib/Sema/SemaDeclCXX.cpp| 209 ++
 clang/lib/Sema/SemaExceptionSpec.cpp  |   7 +-
 clang/lib/Sema/SemaLambda.cpp |   1 +
 clang/lib/Sema/SemaStmt.cpp   |  14 +-
 clang/lib/Sema/SemaTemplate.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  40 +++-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  32 ++-
 clang/lib/Sema/SemaTemplateVariadic.cpp   |  73 +-
 clang/lib/Sema/TreeTransform.h|   7 +
 clang/lib/Serialization/ASTReaderStmt.cpp |  11 +
 clang/lib/Serialization/ASTWriter.cpp |   1 +
 clang/lib/Serialization/ASTWriterStmt.cpp |  11 +
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp  |   1 +
 clang/test/Parser/cxx2c-binding-pack.cpp  |   7 +
 .../cxx2c-binding-pack-nontemplate.cpp|  12 +
 clang/test/SemaCXX/cxx2c-binding-pack.cpp |  82 +++
 clang/test/SemaCXX/typo-correction-crash.cpp  |   3 +-
 clang/tools/libclang/CXCursor.cpp |   1 +
 45 files changed, 728 insertions(+), 111 deletions(-)
 create mode 100644 clang/test/Parser/cxx2c-binding-pack.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack.cpp

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..bdf6c81732d0bc 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -698,6 +698,10 @@ class ValueDecl : public NamedDecl {
 return const_cast(this)->getPotentiallyDecomposedVarDecl();
   }
 
+  /// Determine whether this value is actually a function parameter pack,
+  /// init-capture pack, or structured binding pack
+  bool isParameterPack() const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
@@ -1527,10 +1531,6 @@ class VarDecl : public DeclaratorDecl, public 
Redeclarable {
 NonParmVarDeclBits.IsInitCapture = IC;
   }
 
-  /// Determine whether this variable is actually a function parameter pack or
-  /// init-capture pack.
-  bool isParameterPack() const;
-
   /// Whether this local extern variable declaration's previous declaration
   /// was declared in the same block scope. Only correct in C++.
   bool isPreviousDeclInSameBlockScope() const {
diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index c232556edeff70..12002db17fb3ad 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4131,8 +4131,9 @@ class BindingDecl : public ValueDecl {
   /// binding).
   Expr *Binding = nullptr;
 
-  BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id)
-  : ValueDecl(Decl::Binding, DC, IdLoc, Id, QualType()) {}
+  BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id,
+  QualType T)
+  : ValueDecl(Decl::Binding, DC, IdLoc, Id, T) {}
 
   void anchor() override;
 
@@ -4140,7 +4141,8 @@ class BindingDecl : public ValueDecl {
   friend class ASTDeclReader;
 
   static BindingDecl *Create(ASTContext

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/121417

>From 3c81c5bd989f26331917f1401becc1cfa7f4a454 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Thu, 22 Jul 2021 16:46:33 -0700
Subject: [PATCH 1/5] [Clang][P1061] stuctured binding packs

---
 clang/include/clang/AST/Decl.h|   8 +-
 clang/include/clang/AST/DeclCXX.h |  22 +-
 clang/include/clang/AST/ExprCXX.h |  48 
 clang/include/clang/AST/RecursiveASTVisitor.h |   1 +
 .../clang/Basic/DiagnosticParseKinds.td   |   5 +
 .../clang/Basic/DiagnosticSemaKinds.td|   3 +
 clang/include/clang/Basic/StmtNodes.td|   1 +
 clang/include/clang/Sema/DeclSpec.h   |   1 +
 clang/include/clang/Sema/Sema.h   |   4 +-
 .../include/clang/Serialization/ASTBitCodes.h |   1 +
 clang/lib/AST/ASTContext.cpp  |  14 +-
 clang/lib/AST/ASTImporter.cpp |   2 +-
 clang/lib/AST/Decl.cpp|  11 +-
 clang/lib/AST/DeclBase.cpp|   6 +-
 clang/lib/AST/DeclCXX.cpp |  60 -
 clang/lib/AST/Expr.cpp|   5 +
 clang/lib/AST/ExprCXX.cpp |  48 
 clang/lib/AST/ExprClassification.cpp  |   7 +
 clang/lib/AST/ExprConstant.cpp|   5 +-
 clang/lib/AST/ItaniumMangle.cpp   |   2 +-
 clang/lib/AST/StmtPrinter.cpp |  11 +
 clang/lib/AST/StmtProfile.cpp |   4 +
 clang/lib/CodeGen/CGDebugInfo.cpp |   4 +-
 clang/lib/CodeGen/CGDecl.cpp  |   5 +-
 clang/lib/CodeGen/CodeGenModule.cpp   |   4 +-
 clang/lib/Parse/ParseDecl.cpp |  28 ++-
 clang/lib/Sema/SemaDecl.cpp   |  10 +
 clang/lib/Sema/SemaDeclCXX.cpp| 209 ++
 clang/lib/Sema/SemaExceptionSpec.cpp  |   7 +-
 clang/lib/Sema/SemaLambda.cpp |   1 +
 clang/lib/Sema/SemaStmt.cpp   |  14 +-
 clang/lib/Sema/SemaTemplate.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  40 +++-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  32 ++-
 clang/lib/Sema/SemaTemplateVariadic.cpp   |  73 +-
 clang/lib/Sema/TreeTransform.h|   7 +
 clang/lib/Serialization/ASTReaderStmt.cpp |  11 +
 clang/lib/Serialization/ASTWriter.cpp |   1 +
 clang/lib/Serialization/ASTWriterStmt.cpp |  11 +
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp  |   1 +
 clang/test/Parser/cxx2c-binding-pack.cpp  |   7 +
 .../cxx2c-binding-pack-nontemplate.cpp|  12 +
 clang/test/SemaCXX/cxx2c-binding-pack.cpp |  82 +++
 clang/test/SemaCXX/typo-correction-crash.cpp  |   3 +-
 clang/tools/libclang/CXCursor.cpp |   1 +
 45 files changed, 728 insertions(+), 111 deletions(-)
 create mode 100644 clang/test/Parser/cxx2c-binding-pack.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack.cpp

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..bdf6c81732d0bc 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -698,6 +698,10 @@ class ValueDecl : public NamedDecl {
 return const_cast(this)->getPotentiallyDecomposedVarDecl();
   }
 
+  /// Determine whether this value is actually a function parameter pack,
+  /// init-capture pack, or structured binding pack
+  bool isParameterPack() const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
@@ -1527,10 +1531,6 @@ class VarDecl : public DeclaratorDecl, public 
Redeclarable {
 NonParmVarDeclBits.IsInitCapture = IC;
   }
 
-  /// Determine whether this variable is actually a function parameter pack or
-  /// init-capture pack.
-  bool isParameterPack() const;
-
   /// Whether this local extern variable declaration's previous declaration
   /// was declared in the same block scope. Only correct in C++.
   bool isPreviousDeclInSameBlockScope() const {
diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index c232556edeff70..12002db17fb3ad 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4131,8 +4131,9 @@ class BindingDecl : public ValueDecl {
   /// binding).
   Expr *Binding = nullptr;
 
-  BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id)
-  : ValueDecl(Decl::Binding, DC, IdLoc, Id, QualType()) {}
+  BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id,
+  QualType T)
+  : ValueDecl(Decl::Binding, DC, IdLoc, Id, T) {}
 
   void anchor() override;
 
@@ -4140,7 +4141,8 @@ class BindingDecl : public ValueDecl {
   friend class ASTDeclReader;
 
   static BindingDecl *Create(ASTContext

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -3395,26 +3395,37 @@ VarDecl *ValueDecl::getPotentiallyDecomposedVarDecl() {
   if (auto *Var = llvm::dyn_cast(this))
 return Var;
   if (auto *BD = llvm::dyn_cast(this))
-return llvm::dyn_cast(BD->getDecomposedDecl());
+return llvm::dyn_cast_or_null(BD->getDecomposedDecl());
   return nullptr;
 }
 
 void BindingDecl::anchor() {}
 
 BindingDecl *BindingDecl::Create(ASTContext &C, DeclContext *DC,
- SourceLocation IdLoc, IdentifierInfo *Id) {
-  return new (C, DC) BindingDecl(DC, IdLoc, Id);
+ SourceLocation IdLoc, IdentifierInfo *Id,
+ QualType T) {
+  return new (C, DC) BindingDecl(DC, IdLoc, Id, T);
 }
 
 BindingDecl *BindingDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID) {
-  return new (C, ID) BindingDecl(nullptr, SourceLocation(), nullptr);
+  return new (C, ID)
+  BindingDecl(nullptr, SourceLocation(), nullptr, QualType());
 }
 
 VarDecl *BindingDecl::getHoldingVar() const {
   Expr *B = getBinding();
   if (!B)
 return nullptr;
-  auto *DRE = dyn_cast(B->IgnoreImplicit());
+  return getHoldingVar(B);
+}
+
+VarDecl *BindingDecl::getHoldingVar(Expr *E) {
+  auto *DRE = dyn_cast(E->IgnoreImplicit());

ricejasonf wrote:

This one was like this before. Looking around I see that we call setBinding 
with a call to BuildFieldReferenceExpr which may return a MemberExpr.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -5397,6 +5393,13 @@ bool ValueDecl::isInitCapture() const {
   return false;
 }
 
+bool ValueDecl::isParameterPack() const {
+  if (const auto *NTTP = dyn_cast(this))
+return NTTP->isParameterPack();
+
+  return isa_and_nonnull(getType().getTypePtrOrNull());

ricejasonf wrote:

Adding the assert had it being triggered from a lot of tests so I am removing 
it for now.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Fix template arguments in local classes (PR #121225)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf created 
https://github.com/llvm/llvm-project/pull/121225

In the development of P1061 (Structured Bindings Introduce a Patch), I found 
this bug in the template instantiation of a
local class. The issue is caused by the instantiation of the original template 
and not the partially instantiated template. In
the example (sans the fix) the instantiation uses the first template parameter 
from the previous instantiation and not the current one so the error hits an 
assertion when it is expecting an NTTP. If they were both types then it might 
gladly accept the type from the wrong template which is kind of scary.

In the test, the reference to `i` is substituted with a placeholder AST object 
that represents the resolved value when instantiating `g`. However, since the 
old template is used, the instantiation sees an AST object that only contains 
the template argument index in the context of instantiating the lambda which 
has a type template parameter (ie auto).

I question if we should use `getTemplateInstantiationPattern` at all here. 
Other errors involving local classes in nested templates could also be caused 
by the misuse of this function (because it gets the uninstantiated template).


>From f1058d13315682b8bd6c3ac06a3225060d11ec61 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Mon, 5 Aug 2024 13:53:33 -0700
Subject: [PATCH] [Clang][P1061] Fix template arguments in local classes

---
 clang/lib/Sema/SemaTemplateInstantiate.cpp |  6 +-
 .../SemaCXX/local-class-template-param-crash.cpp   | 14 ++
 2 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/local-class-template-param-crash.cpp

diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index fb0f38df62a744..baa5ff35295349 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -4433,8 +4433,12 @@ Sema::InstantiateClassMembers(SourceLocation 
PointOfInstantiation,
   // No need to instantiate in-class initializers during explicit
   // instantiation.
   if (Field->hasInClassInitializer() && TSK == TSK_ImplicitInstantiation) {
+// Handle local classes which could have substituted template params.
 CXXRecordDecl *ClassPattern =
-Instantiation->getTemplateInstantiationPattern();
+Instantiation->isLocalClass()
+? Instantiation->getInstantiatedFromMemberClass()
+: Instantiation->getTemplateInstantiationPattern();
+
 DeclContext::lookup_result Lookup =
 ClassPattern->lookup(Field->getDeclName());
 FieldDecl *Pattern = Lookup.find_first();
diff --git a/clang/test/SemaCXX/local-class-template-param-crash.cpp 
b/clang/test/SemaCXX/local-class-template-param-crash.cpp
new file mode 100644
index 00..ffa8590eaf77d8
--- /dev/null
+++ b/clang/test/SemaCXX/local-class-template-param-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// expected-no-diagnostics
+
+template 
+int g() {
+  return [] (auto) -> int {
+struct L {
+  int m = i;
+};
+return 0;
+  } (42);
+}
+
+int v = g<1>();

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/125394

>From a323e058b2c8adf97f7f9a55a9187f74de9b8d17 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Sun, 2 Feb 2025 00:52:47 -0800
Subject: [PATCH 1/3] [Clang][P1061] Consolidate ResolvedUnexpandedPackExpr
 into FunctionParmPackExpr

---
 clang/include/clang/AST/DeclCXX.h | 28 +
 clang/include/clang/AST/ExprCXX.h | 21 +-
 clang/include/clang/Sema/Template.h   |  2 +-
 clang/lib/AST/DeclCXX.cpp |  9 +++--
 clang/lib/AST/ExprCXX.cpp | 14 +++
 clang/lib/Sema/SemaDeclCXX.cpp| 26 ++--
 clang/lib/Sema/SemaExpr.cpp   |  6 +--
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 30 +-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 40 +++
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 32 +--
 clang/lib/Serialization/ASTReaderStmt.cpp |  6 +--
 clang/test/AST/ast-dump-binding-pack.cpp  | 13 ++
 clang/test/SemaCXX/cxx2c-binding-pack.cpp | 11 +
 13 files changed, 103 insertions(+), 135 deletions(-)

diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 766821b4fb25cb5..1c630d616903550 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4194,8 +4194,8 @@ class BindingDecl : public ValueDecl {
   /// decomposition declaration, and when the initializer is type-dependent.
   Expr *getBinding() const { return Binding; }
 
-  // Get the array of Exprs when the binding represents a pack.
-  llvm::ArrayRef getBindingPackExprs() const;
+  // Get the array of nested BindingDecls when the binding represents a pack.
+  llvm::ArrayRef getBindingPackDecls() const;
 
   /// Get the decomposition declaration that this binding represents a
   /// decomposition of.
@@ -4246,10 +4246,8 @@ class DecompositionDecl final
 for (auto *B : Bindings) {
   B->setDecomposedDecl(this);
   if (B->isParameterPack() && B->getBinding()) {
-for (Expr *E : B->getBindingPackExprs()) {
-  auto *DRE = cast(E);
-  auto *NestedB = cast(DRE->getDecl());
-  NestedB->setDecomposedDecl(this);
+for (BindingDecl *NestedBD : B->getBindingPackDecls()) {
+  NestedBD->setDecomposedDecl(this);
 }
   }
 }
@@ -4278,25 +4276,21 @@ class DecompositionDecl final
   // Provide a flattened range to visit each binding.
   auto flat_bindings() const {
 llvm::ArrayRef Bindings = bindings();
-llvm::ArrayRef PackExprs;
+llvm::ArrayRef PackBindings;
 
 // Split the bindings into subranges split by the pack.
-auto S1 = Bindings.take_until(
+auto BeforePackBindings = Bindings.take_until(
 [](BindingDecl *BD) { return BD->isParameterPack(); });
 
-Bindings = Bindings.drop_front(S1.size());
+Bindings = Bindings.drop_front(BeforePackBindings.size());
 if (!Bindings.empty()) {
-  PackExprs = Bindings.front()->getBindingPackExprs();
+  PackBindings = Bindings.front()->getBindingPackDecls();
   Bindings = Bindings.drop_front();
 }
 
-auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
-  auto *DRE = cast(E);
-  return cast(DRE->getDecl());
-});
-
-return llvm::concat(std::move(S1), std::move(S2),
-   std::move(Bindings));
+return llvm::concat(std::move(BeforePackBindings),
+std::move(PackBindings),
+std::move(Bindings));
   }
 
   void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
diff --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 0b6c8cfb163c958..2b23fa51c6232b2 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4649,13 +4649,13 @@ class SubstNonTypeTemplateParmPackExpr : public Expr {
 /// \endcode
 class FunctionParmPackExpr final
 : public Expr,
-  private llvm::TrailingObjects {
+  private llvm::TrailingObjects {
   friend class ASTReader;
   friend class ASTStmtReader;
   friend TrailingObjects;
 
   /// The function parameter pack which was referenced.
-  VarDecl *ParamPack;
+  ValueDecl *ParamPack;
 
   /// The location of the function parameter pack reference.
   SourceLocation NameLoc;
@@ -4663,35 +4663,34 @@ class FunctionParmPackExpr final
   /// The number of expansions of this pack.
   unsigned NumParameters;
 
-  FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
-   SourceLocation NameLoc, unsigned NumParams,
-   VarDecl *const *Params);
+  FunctionParmPackExpr(QualType T, ValueDecl *ParamPack, SourceLocation 
NameLoc,
+   unsigned NumParams, ValueDecl *const *Params);
 
 public:
   static FunctionParmPackExpr *Create(const ASTContext &Context, QualType T,
-   

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

ricejasonf wrote:

> Can i merge this for you?

@cor3ntin Whenever you think it is ready, yes please. I know Erich did not want 
the C-style cast.

https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

@@ -4278,25 +4276,21 @@ class DecompositionDecl final
   // Provide a flattened range to visit each binding.
   auto flat_bindings() const {
 llvm::ArrayRef Bindings = bindings();
-llvm::ArrayRef PackExprs;
+llvm::ArrayRef PackBindings;
 
 // Split the bindings into subranges split by the pack.
-auto S1 = Bindings.take_until(
+auto BeforePackBindings = Bindings.take_until(
 [](BindingDecl *BD) { return BD->isParameterPack(); });
 
-Bindings = Bindings.drop_front(S1.size());
+Bindings = Bindings.drop_front(BeforePackBindings.size());
 if (!Bindings.empty()) {
-  PackExprs = Bindings.front()->getBindingPackExprs();
+  PackBindings = Bindings.front()->getBindingPackDecls();
   Bindings = Bindings.drop_front();
 }
 
-auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
-  auto *DRE = cast(E);
-  return cast(DRE->getDecl());
-});
-
-return llvm::concat(std::move(S1), std::move(S2),
-   std::move(Bindings));
+return llvm::concat(std::move(BeforePackBindings),

ricejasonf wrote:

They add... rvalue don't they? :rofl: . I added them to work around the 
dangling references.

https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

@@ -1197,25 +1197,19 @@ Decl 
*TemplateDeclInstantiator::VisitDecompositionDecl(DecompositionDecl *D) {
 for (auto *NewBD : NewBindings)
   NewBD->setInvalidDecl();
 
-  if (OldResolvedPack) {
-// Mark the holding vars (if any) in the pack as instantiated since
-// they are created implicitly.
+  if (OldBindingPack) {
+// Mark the bindings in the pack as instantiated.
 auto Bindings = NewDD->bindings();
-auto BPack = llvm::find_if(
+BindingDecl *NewBindingPack = *llvm::find_if(
 Bindings, [](BindingDecl *D) -> bool { return D->isParameterPack(); });

ricejasonf wrote:

What do you think about capturing the index of the old pack in the loop above 
and using that instead of searching?

https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/125394

>From a323e058b2c8adf97f7f9a55a9187f74de9b8d17 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Sun, 2 Feb 2025 00:52:47 -0800
Subject: [PATCH 1/5] [Clang][P1061] Consolidate ResolvedUnexpandedPackExpr
 into FunctionParmPackExpr

---
 clang/include/clang/AST/DeclCXX.h | 28 +
 clang/include/clang/AST/ExprCXX.h | 21 +-
 clang/include/clang/Sema/Template.h   |  2 +-
 clang/lib/AST/DeclCXX.cpp |  9 +++--
 clang/lib/AST/ExprCXX.cpp | 14 +++
 clang/lib/Sema/SemaDeclCXX.cpp| 26 ++--
 clang/lib/Sema/SemaExpr.cpp   |  6 +--
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 30 +-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 40 +++
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 32 +--
 clang/lib/Serialization/ASTReaderStmt.cpp |  6 +--
 clang/test/AST/ast-dump-binding-pack.cpp  | 13 ++
 clang/test/SemaCXX/cxx2c-binding-pack.cpp | 11 +
 13 files changed, 103 insertions(+), 135 deletions(-)

diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 766821b4fb25cb5..1c630d616903550 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4194,8 +4194,8 @@ class BindingDecl : public ValueDecl {
   /// decomposition declaration, and when the initializer is type-dependent.
   Expr *getBinding() const { return Binding; }
 
-  // Get the array of Exprs when the binding represents a pack.
-  llvm::ArrayRef getBindingPackExprs() const;
+  // Get the array of nested BindingDecls when the binding represents a pack.
+  llvm::ArrayRef getBindingPackDecls() const;
 
   /// Get the decomposition declaration that this binding represents a
   /// decomposition of.
@@ -4246,10 +4246,8 @@ class DecompositionDecl final
 for (auto *B : Bindings) {
   B->setDecomposedDecl(this);
   if (B->isParameterPack() && B->getBinding()) {
-for (Expr *E : B->getBindingPackExprs()) {
-  auto *DRE = cast(E);
-  auto *NestedB = cast(DRE->getDecl());
-  NestedB->setDecomposedDecl(this);
+for (BindingDecl *NestedBD : B->getBindingPackDecls()) {
+  NestedBD->setDecomposedDecl(this);
 }
   }
 }
@@ -4278,25 +4276,21 @@ class DecompositionDecl final
   // Provide a flattened range to visit each binding.
   auto flat_bindings() const {
 llvm::ArrayRef Bindings = bindings();
-llvm::ArrayRef PackExprs;
+llvm::ArrayRef PackBindings;
 
 // Split the bindings into subranges split by the pack.
-auto S1 = Bindings.take_until(
+auto BeforePackBindings = Bindings.take_until(
 [](BindingDecl *BD) { return BD->isParameterPack(); });
 
-Bindings = Bindings.drop_front(S1.size());
+Bindings = Bindings.drop_front(BeforePackBindings.size());
 if (!Bindings.empty()) {
-  PackExprs = Bindings.front()->getBindingPackExprs();
+  PackBindings = Bindings.front()->getBindingPackDecls();
   Bindings = Bindings.drop_front();
 }
 
-auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
-  auto *DRE = cast(E);
-  return cast(DRE->getDecl());
-});
-
-return llvm::concat(std::move(S1), std::move(S2),
-   std::move(Bindings));
+return llvm::concat(std::move(BeforePackBindings),
+std::move(PackBindings),
+std::move(Bindings));
   }
 
   void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
diff --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 0b6c8cfb163c958..2b23fa51c6232b2 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4649,13 +4649,13 @@ class SubstNonTypeTemplateParmPackExpr : public Expr {
 /// \endcode
 class FunctionParmPackExpr final
 : public Expr,
-  private llvm::TrailingObjects {
+  private llvm::TrailingObjects {
   friend class ASTReader;
   friend class ASTStmtReader;
   friend TrailingObjects;
 
   /// The function parameter pack which was referenced.
-  VarDecl *ParamPack;
+  ValueDecl *ParamPack;
 
   /// The location of the function parameter pack reference.
   SourceLocation NameLoc;
@@ -4663,35 +4663,34 @@ class FunctionParmPackExpr final
   /// The number of expansions of this pack.
   unsigned NumParameters;
 
-  FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
-   SourceLocation NameLoc, unsigned NumParams,
-   VarDecl *const *Params);
+  FunctionParmPackExpr(QualType T, ValueDecl *ParamPack, SourceLocation 
NameLoc,
+   unsigned NumParams, ValueDecl *const *Params);
 
 public:
   static FunctionParmPackExpr *Create(const ASTContext &Context, QualType T,
-   

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

@@ -3492,10 +3492,13 @@ VarDecl *BindingDecl::getHoldingVar() const {
   return VD;
 }
 
-llvm::ArrayRef BindingDecl::getBindingPackExprs() const {
+llvm::ArrayRef BindingDecl::getBindingPackDecls() const {
   assert(Binding && "expecting a pack expr");
-  auto *RP = cast(Binding);
-  return RP->getExprs();
+  auto *FP = cast(Binding);
+  ValueDecl *const *First = FP->getNumExpansions() > 0 ? FP->begin() : nullptr;
+  assert((!First || isa(*First)) && "expecting a BindingDecl");
+  return llvm::ArrayRef(
+  reinterpret_cast(First), FP->getNumExpansions());

ricejasonf wrote:

`llvm::cast` will not return a reference to the pointer so it cannot be used 
for the ArrayRef.

https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Do not try to transform invalid bindings (PR #125658)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/125658
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

@@ -124,6 +125,14 @@ void lambda_capture() {
   [&x...] { (void)sum(x...); }();
 }
 
+struct S2 {
+int a, b, c;
+};
+
+auto X = []  () {

ricejasonf wrote:

Oh, sorry, I added your test with `clsss` not realizing you were talking about 
modifying an existing test. :sweat_smile: 

https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

@@ -3492,10 +3492,13 @@ VarDecl *BindingDecl::getHoldingVar() const {
   return VD;
 }
 
-llvm::ArrayRef BindingDecl::getBindingPackExprs() const {
+llvm::ArrayRef BindingDecl::getBindingPackDecls() const {
   assert(Binding && "expecting a pack expr");
-  auto *RP = cast(Binding);
-  return RP->getExprs();
+  auto *FP = cast(Binding);
+  ValueDecl *const *First = FP->getNumExpansions() > 0 ? FP->begin() : nullptr;
+  assert((!First || isa(*First)) && "expecting a BindingDecl");
+  return llvm::ArrayRef((BindingDecl *const *)First,

ricejasonf wrote:

It says you marked this as resolved.

https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/125394

>From a323e058b2c8adf97f7f9a55a9187f74de9b8d17 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Sun, 2 Feb 2025 00:52:47 -0800
Subject: [PATCH 1/3] [Clang][P1061] Consolidate ResolvedUnexpandedPackExpr
 into FunctionParmPackExpr

---
 clang/include/clang/AST/DeclCXX.h | 28 +
 clang/include/clang/AST/ExprCXX.h | 21 +-
 clang/include/clang/Sema/Template.h   |  2 +-
 clang/lib/AST/DeclCXX.cpp |  9 +++--
 clang/lib/AST/ExprCXX.cpp | 14 +++
 clang/lib/Sema/SemaDeclCXX.cpp| 26 ++--
 clang/lib/Sema/SemaExpr.cpp   |  6 +--
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 30 +-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 40 +++
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 32 +--
 clang/lib/Serialization/ASTReaderStmt.cpp |  6 +--
 clang/test/AST/ast-dump-binding-pack.cpp  | 13 ++
 clang/test/SemaCXX/cxx2c-binding-pack.cpp | 11 +
 13 files changed, 103 insertions(+), 135 deletions(-)

diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 766821b4fb25cb..1c630d61690355 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4194,8 +4194,8 @@ class BindingDecl : public ValueDecl {
   /// decomposition declaration, and when the initializer is type-dependent.
   Expr *getBinding() const { return Binding; }
 
-  // Get the array of Exprs when the binding represents a pack.
-  llvm::ArrayRef getBindingPackExprs() const;
+  // Get the array of nested BindingDecls when the binding represents a pack.
+  llvm::ArrayRef getBindingPackDecls() const;
 
   /// Get the decomposition declaration that this binding represents a
   /// decomposition of.
@@ -4246,10 +4246,8 @@ class DecompositionDecl final
 for (auto *B : Bindings) {
   B->setDecomposedDecl(this);
   if (B->isParameterPack() && B->getBinding()) {
-for (Expr *E : B->getBindingPackExprs()) {
-  auto *DRE = cast(E);
-  auto *NestedB = cast(DRE->getDecl());
-  NestedB->setDecomposedDecl(this);
+for (BindingDecl *NestedBD : B->getBindingPackDecls()) {
+  NestedBD->setDecomposedDecl(this);
 }
   }
 }
@@ -4278,25 +4276,21 @@ class DecompositionDecl final
   // Provide a flattened range to visit each binding.
   auto flat_bindings() const {
 llvm::ArrayRef Bindings = bindings();
-llvm::ArrayRef PackExprs;
+llvm::ArrayRef PackBindings;
 
 // Split the bindings into subranges split by the pack.
-auto S1 = Bindings.take_until(
+auto BeforePackBindings = Bindings.take_until(
 [](BindingDecl *BD) { return BD->isParameterPack(); });
 
-Bindings = Bindings.drop_front(S1.size());
+Bindings = Bindings.drop_front(BeforePackBindings.size());
 if (!Bindings.empty()) {
-  PackExprs = Bindings.front()->getBindingPackExprs();
+  PackBindings = Bindings.front()->getBindingPackDecls();
   Bindings = Bindings.drop_front();
 }
 
-auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
-  auto *DRE = cast(E);
-  return cast(DRE->getDecl());
-});
-
-return llvm::concat(std::move(S1), std::move(S2),
-   std::move(Bindings));
+return llvm::concat(std::move(BeforePackBindings),
+std::move(PackBindings),
+std::move(Bindings));
   }
 
   void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
diff --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 0b6c8cfb163c95..2b23fa51c6232b 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4649,13 +4649,13 @@ class SubstNonTypeTemplateParmPackExpr : public Expr {
 /// \endcode
 class FunctionParmPackExpr final
 : public Expr,
-  private llvm::TrailingObjects {
+  private llvm::TrailingObjects {
   friend class ASTReader;
   friend class ASTStmtReader;
   friend TrailingObjects;
 
   /// The function parameter pack which was referenced.
-  VarDecl *ParamPack;
+  ValueDecl *ParamPack;
 
   /// The location of the function parameter pack reference.
   SourceLocation NameLoc;
@@ -4663,35 +4663,34 @@ class FunctionParmPackExpr final
   /// The number of expansions of this pack.
   unsigned NumParameters;
 
-  FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
-   SourceLocation NameLoc, unsigned NumParams,
-   VarDecl *const *Params);
+  FunctionParmPackExpr(QualType T, ValueDecl *ParamPack, SourceLocation 
NameLoc,
+   unsigned NumParams, ValueDecl *const *Params);
 
 public:
   static FunctionParmPackExpr *Create(const ASTContext &Context, QualType T,
-   

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

ricejasonf wrote:

@Sirraide I did try it and the bug persists. The `LHS` is a RecoveryExpr, so I 
am not sure how to approach that.

https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

@@ -3492,10 +3492,13 @@ VarDecl *BindingDecl::getHoldingVar() const {
   return VD;
 }
 
-llvm::ArrayRef BindingDecl::getBindingPackExprs() const {
+llvm::ArrayRef BindingDecl::getBindingPackDecls() const {
   assert(Binding && "expecting a pack expr");
-  auto *RP = cast(Binding);
-  return RP->getExprs();
+  auto *FP = cast(Binding);
+  ValueDecl *const *First = FP->getNumExpansions() > 0 ? FP->begin() : nullptr;
+  assert((!First || isa(*First)) && "expecting a BindingDecl");
+  return llvm::ArrayRef((BindingDecl *const *)First,
+   FP->getNumExpansions());

ricejasonf wrote:

Ah, I had something like that, but I didn't think to `cast` to a pointer type 
(or know that you could like that).

https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/125394

>From a323e058b2c8adf97f7f9a55a9187f74de9b8d17 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Sun, 2 Feb 2025 00:52:47 -0800
Subject: [PATCH 1/2] [Clang][P1061] Consolidate ResolvedUnexpandedPackExpr
 into FunctionParmPackExpr

---
 clang/include/clang/AST/DeclCXX.h | 28 +
 clang/include/clang/AST/ExprCXX.h | 21 +-
 clang/include/clang/Sema/Template.h   |  2 +-
 clang/lib/AST/DeclCXX.cpp |  9 +++--
 clang/lib/AST/ExprCXX.cpp | 14 +++
 clang/lib/Sema/SemaDeclCXX.cpp| 26 ++--
 clang/lib/Sema/SemaExpr.cpp   |  6 +--
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 30 +-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 40 +++
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 32 +--
 clang/lib/Serialization/ASTReaderStmt.cpp |  6 +--
 clang/test/AST/ast-dump-binding-pack.cpp  | 13 ++
 clang/test/SemaCXX/cxx2c-binding-pack.cpp | 11 +
 13 files changed, 103 insertions(+), 135 deletions(-)

diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 766821b4fb25cb..1c630d61690355 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4194,8 +4194,8 @@ class BindingDecl : public ValueDecl {
   /// decomposition declaration, and when the initializer is type-dependent.
   Expr *getBinding() const { return Binding; }
 
-  // Get the array of Exprs when the binding represents a pack.
-  llvm::ArrayRef getBindingPackExprs() const;
+  // Get the array of nested BindingDecls when the binding represents a pack.
+  llvm::ArrayRef getBindingPackDecls() const;
 
   /// Get the decomposition declaration that this binding represents a
   /// decomposition of.
@@ -4246,10 +4246,8 @@ class DecompositionDecl final
 for (auto *B : Bindings) {
   B->setDecomposedDecl(this);
   if (B->isParameterPack() && B->getBinding()) {
-for (Expr *E : B->getBindingPackExprs()) {
-  auto *DRE = cast(E);
-  auto *NestedB = cast(DRE->getDecl());
-  NestedB->setDecomposedDecl(this);
+for (BindingDecl *NestedBD : B->getBindingPackDecls()) {
+  NestedBD->setDecomposedDecl(this);
 }
   }
 }
@@ -4278,25 +4276,21 @@ class DecompositionDecl final
   // Provide a flattened range to visit each binding.
   auto flat_bindings() const {
 llvm::ArrayRef Bindings = bindings();
-llvm::ArrayRef PackExprs;
+llvm::ArrayRef PackBindings;
 
 // Split the bindings into subranges split by the pack.
-auto S1 = Bindings.take_until(
+auto BeforePackBindings = Bindings.take_until(
 [](BindingDecl *BD) { return BD->isParameterPack(); });
 
-Bindings = Bindings.drop_front(S1.size());
+Bindings = Bindings.drop_front(BeforePackBindings.size());
 if (!Bindings.empty()) {
-  PackExprs = Bindings.front()->getBindingPackExprs();
+  PackBindings = Bindings.front()->getBindingPackDecls();
   Bindings = Bindings.drop_front();
 }
 
-auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
-  auto *DRE = cast(E);
-  return cast(DRE->getDecl());
-});
-
-return llvm::concat(std::move(S1), std::move(S2),
-   std::move(Bindings));
+return llvm::concat(std::move(BeforePackBindings),
+std::move(PackBindings),
+std::move(Bindings));
   }
 
   void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
diff --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 0b6c8cfb163c95..2b23fa51c6232b 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4649,13 +4649,13 @@ class SubstNonTypeTemplateParmPackExpr : public Expr {
 /// \endcode
 class FunctionParmPackExpr final
 : public Expr,
-  private llvm::TrailingObjects {
+  private llvm::TrailingObjects {
   friend class ASTReader;
   friend class ASTStmtReader;
   friend TrailingObjects;
 
   /// The function parameter pack which was referenced.
-  VarDecl *ParamPack;
+  ValueDecl *ParamPack;
 
   /// The location of the function parameter pack reference.
   SourceLocation NameLoc;
@@ -4663,35 +4663,34 @@ class FunctionParmPackExpr final
   /// The number of expansions of this pack.
   unsigned NumParameters;
 
-  FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
-   SourceLocation NameLoc, unsigned NumParams,
-   VarDecl *const *Params);
+  FunctionParmPackExpr(QualType T, ValueDecl *ParamPack, SourceLocation 
NameLoc,
+   unsigned NumParams, ValueDecl *const *Params);
 
 public:
   static FunctionParmPackExpr *Create(const ASTContext &Context, QualType T,
-   

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

ricejasonf wrote:

@cor3ntin Sorry, I tried "Commit suggestion", but it didn't look like you can 
cast like that so I just reverted.

https://github.com/llvm/llvm-project/pull/125394
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Consolidate ResolvedUnpexandedPackExpr into FunctionParmPackExpr (PR #125394)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf created 
https://github.com/llvm/llvm-project/pull/125394

This merges the functionality of ResolvedUnexpandedPackExpr into 
FunctionParmPackExpr. I also added a test to show that 
https://github.com/llvm/llvm-project/issues/125103 should be fixed with this. I 
put the removal of ResolvedUnexpandedPackExpr in its own commit. Let me know 
what you think.

@cor3ntin 

>From a323e058b2c8adf97f7f9a55a9187f74de9b8d17 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Sun, 2 Feb 2025 00:52:47 -0800
Subject: [PATCH 1/2] [Clang][P1061] Consolidate ResolvedUnexpandedPackExpr
 into FunctionParmPackExpr

---
 clang/include/clang/AST/DeclCXX.h | 28 +
 clang/include/clang/AST/ExprCXX.h | 21 +-
 clang/include/clang/Sema/Template.h   |  2 +-
 clang/lib/AST/DeclCXX.cpp |  9 +++--
 clang/lib/AST/ExprCXX.cpp | 14 +++
 clang/lib/Sema/SemaDeclCXX.cpp| 26 ++--
 clang/lib/Sema/SemaExpr.cpp   |  6 +--
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 30 +-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  | 40 +++
 clang/lib/Sema/SemaTemplateVariadic.cpp   | 32 +--
 clang/lib/Serialization/ASTReaderStmt.cpp |  6 +--
 clang/test/AST/ast-dump-binding-pack.cpp  | 13 ++
 clang/test/SemaCXX/cxx2c-binding-pack.cpp | 11 +
 13 files changed, 103 insertions(+), 135 deletions(-)

diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 766821b4fb25cb..1c630d61690355 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4194,8 +4194,8 @@ class BindingDecl : public ValueDecl {
   /// decomposition declaration, and when the initializer is type-dependent.
   Expr *getBinding() const { return Binding; }
 
-  // Get the array of Exprs when the binding represents a pack.
-  llvm::ArrayRef getBindingPackExprs() const;
+  // Get the array of nested BindingDecls when the binding represents a pack.
+  llvm::ArrayRef getBindingPackDecls() const;
 
   /// Get the decomposition declaration that this binding represents a
   /// decomposition of.
@@ -4246,10 +4246,8 @@ class DecompositionDecl final
 for (auto *B : Bindings) {
   B->setDecomposedDecl(this);
   if (B->isParameterPack() && B->getBinding()) {
-for (Expr *E : B->getBindingPackExprs()) {
-  auto *DRE = cast(E);
-  auto *NestedB = cast(DRE->getDecl());
-  NestedB->setDecomposedDecl(this);
+for (BindingDecl *NestedBD : B->getBindingPackDecls()) {
+  NestedBD->setDecomposedDecl(this);
 }
   }
 }
@@ -4278,25 +4276,21 @@ class DecompositionDecl final
   // Provide a flattened range to visit each binding.
   auto flat_bindings() const {
 llvm::ArrayRef Bindings = bindings();
-llvm::ArrayRef PackExprs;
+llvm::ArrayRef PackBindings;
 
 // Split the bindings into subranges split by the pack.
-auto S1 = Bindings.take_until(
+auto BeforePackBindings = Bindings.take_until(
 [](BindingDecl *BD) { return BD->isParameterPack(); });
 
-Bindings = Bindings.drop_front(S1.size());
+Bindings = Bindings.drop_front(BeforePackBindings.size());
 if (!Bindings.empty()) {
-  PackExprs = Bindings.front()->getBindingPackExprs();
+  PackBindings = Bindings.front()->getBindingPackDecls();
   Bindings = Bindings.drop_front();
 }
 
-auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
-  auto *DRE = cast(E);
-  return cast(DRE->getDecl());
-});
-
-return llvm::concat(std::move(S1), std::move(S2),
-   std::move(Bindings));
+return llvm::concat(std::move(BeforePackBindings),
+std::move(PackBindings),
+std::move(Bindings));
   }
 
   void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
diff --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 0b6c8cfb163c95..2b23fa51c6232b 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -4649,13 +4649,13 @@ class SubstNonTypeTemplateParmPackExpr : public Expr {
 /// \endcode
 class FunctionParmPackExpr final
 : public Expr,
-  private llvm::TrailingObjects {
+  private llvm::TrailingObjects {
   friend class ASTReader;
   friend class ASTStmtReader;
   friend TrailingObjects;
 
   /// The function parameter pack which was referenced.
-  VarDecl *ParamPack;
+  ValueDecl *ParamPack;
 
   /// The location of the function parameter pack reference.
   SourceLocation NameLoc;
@@ -4663,35 +4663,34 @@ class FunctionParmPackExpr final
   /// The number of expansions of this pack.
   unsigned NumParameters;
 
-  FunctionParmPackExpr(QualType T, VarDecl *ParamPack,
-   SourceLocation NameLoc, unsigned NumParams

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -12726,11 +12726,15 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
 
   // Likewise, variables with tuple-like bindings are required if their
   // bindings have side-effects.
-  if (const auto *DD = dyn_cast(VD))
-for (const auto *BD : DD->bindings())
-  if (const auto *BindingVD = BD->getHoldingVar())
-if (DeclMustBeEmitted(BindingVD))
-  return true;
+  if (const auto *DD = dyn_cast(VD)) {
+bool BindingResult = false;
+DD->VisitHoldingVars([&](VarDecl *BindingVD) {

ricejasonf wrote:

I just added a flat_binding_iterator to replace these Visit* functions. I 
forgot to use it here, but if a custom iterator is what you wanted, please 
consider it.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -2202,6 +2202,17 @@ void 
ASTStmtWriter::VisitPackIndexingExpr(PackIndexingExpr *E) {
   Code = serialization::EXPR_PACK_INDEXING;
 }
 
+void ASTStmtWriter::VisitResolvedUnexpandedPackExpr(

ricejasonf wrote:

Yes, and thank you for the heads up. I did add a test that does this.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -951,28 +959,124 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator 
&D,
   return New;
 }
 
+// CheckBindingsCount
+//  - Checks the arity of the structured bindings
+//  - Creates the resolved pack expr if there is
+//one
+static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
+   QualType DecompType,
+   ArrayRef Bindings,
+   unsigned MemberCount) {
+  auto BindingWithPackItr =
+  std::find_if(Bindings.begin(), Bindings.end(),
+   [](BindingDecl *D) -> bool { return D->isParameterPack(); 
});
+  bool HasPack = BindingWithPackItr != Bindings.end();
+  bool IsValid;
+  if (!HasPack) {
+IsValid = Bindings.size() == MemberCount;
+  } else {
+// there may not be more members than non-pack bindings
+IsValid = MemberCount >= Bindings.size() - 1;
+  }
+
+  if (IsValid && HasPack) {
+// create the pack expr and assign it to the binding
+unsigned PackSize = MemberCount - Bindings.size() + 1;
+QualType PackType = S.Context.getPackExpansionType(
+S.Context.DependentTy, std::nullopt, /*ExpectsPackInType=*/false);
+BindingDecl *BD = (*BindingWithPackItr);
+BD->setBinding(PackType,
+   ResolvedUnexpandedPackExpr::Create(
+   S.Context, DD->getBeginLoc(), DecompType, PackSize));

ricejasonf wrote:

Oh yeah, using `flat_bindings` there looks doable if we put the 
ResolvedUnexpandedPack in there ahead of it like that.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -5321,6 +5321,59 @@ class BuiltinBitCastExpr final
   }
 };
 
+// Represents an unexpanded pack where the list of expressions are
+// known. These are used when structured bindings introduce a pack.
+class ResolvedUnexpandedPackExpr final
+: public Expr,
+  private llvm::TrailingObjects {

ricejasonf wrote:

Personally, I am content to leave it as is, but storing Decls instead of Exprs 
does have the benefit of delaying building the DeclRefExprs until expansion 
which creates less garbage and would simplify `flat_bindings`. If anything, I 
would leave the name `ResolvedUnexpandedPackExpr` and make the trailing objects 
`ValueDecl`  to support future consolidation. That is premised on never wanting 
to create a pack of expressions which was my original intent as this was made 
for P1221 (which I have abandoned). I will defer to @cor3ntin.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

ricejasonf wrote:

> I wonder if we should rename flat_bindings() to bindings() and bindings() to 
> bindings_as_written() (or similar), as I expect that to be the function we 
> ~always want to use.

I do not have a strong preference, but I kept `bindings()` the same since it 
represents the structure of the AST and that is how it is used in most places. 
(like 2x time more than `flat_bindings()`)

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -5321,6 +5321,59 @@ class BuiltinBitCastExpr final
   }
 };
 
+// Represents an unexpanded pack where the list of expressions are
+// known. These are used when structured bindings introduce a pack.
+class ResolvedUnexpandedPackExpr final

ricejasonf wrote:

I definitely think they could be generalized which is what I had in mind with 
`ResolvedUnexpandedPackExpr`. This is originally what I had for parmexprs 
(P1221).

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -951,28 +959,124 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator 
&D,
   return New;
 }
 
+// CheckBindingsCount
+//  - Checks the arity of the structured bindings
+//  - Creates the resolved pack expr if there is
+//one
+static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
+   QualType DecompType,
+   ArrayRef Bindings,
+   unsigned MemberCount) {
+  auto BindingWithPackItr =
+  std::find_if(Bindings.begin(), Bindings.end(),
+   [](BindingDecl *D) -> bool { return D->isParameterPack(); 
});
+  bool HasPack = BindingWithPackItr != Bindings.end();
+  bool IsValid;
+  if (!HasPack) {
+IsValid = Bindings.size() == MemberCount;
+  } else {
+// there may not be more members than non-pack bindings
+IsValid = MemberCount >= Bindings.size() - 1;
+  }
+
+  if (IsValid && HasPack) {
+// create the pack expr and assign it to the binding
+unsigned PackSize = MemberCount - Bindings.size() + 1;
+QualType PackType = S.Context.getPackExpansionType(
+S.Context.DependentTy, std::nullopt, /*ExpectsPackInType=*/false);
+BindingDecl *BD = (*BindingWithPackItr);
+BD->setBinding(PackType,
+   ResolvedUnexpandedPackExpr::Create(
+   S.Context, DD->getBeginLoc(), DecompType, PackSize));

ricejasonf wrote:

The problem is that we do not know the length of the pack until the initializer 
expression is created. This happens in various locations depending on context 
such as range based for loops. I am not saying it isn't possible, but I think 
it would require creating the init expression up front which I do not know if 
that would be problematic with how variables are checked. We could also do 
Bindings in a separate allocation, but that would require keeping the 
DecompositionDeclarator around somehow, and there would need to be that extra 
information in the DecompositionDecl which would have a cost for Decomps that 
do not have a pack. (I don't know if that is an issue.)

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -951,28 +959,124 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator 
&D,
   return New;
 }
 
+// CheckBindingsCount
+//  - Checks the arity of the structured bindings
+//  - Creates the resolved pack expr if there is
+//one
+static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
+   QualType DecompType,
+   ArrayRef Bindings,
+   unsigned MemberCount) {
+  auto BindingWithPackItr =
+  std::find_if(Bindings.begin(), Bindings.end(),
+   [](BindingDecl *D) -> bool { return D->isParameterPack(); 
});
+  bool HasPack = BindingWithPackItr != Bindings.end();
+  bool IsValid;
+  if (!HasPack) {
+IsValid = Bindings.size() == MemberCount;
+  } else {
+// there may not be more members than non-pack bindings
+IsValid = MemberCount >= Bindings.size() - 1;
+  }
+
+  if (IsValid && HasPack) {
+// create the pack expr and assign it to the binding
+unsigned PackSize = MemberCount - Bindings.size() + 1;
+QualType PackType = S.Context.getPackExpansionType(
+S.Context.DependentTy, std::nullopt, /*ExpectsPackInType=*/false);
+BindingDecl *BD = (*BindingWithPackItr);
+BD->setBinding(PackType,
+   ResolvedUnexpandedPackExpr::Create(
+   S.Context, DD->getBeginLoc(), DecompType, PackSize));

ricejasonf wrote:

What is IF? (initializer first?)

I have gone down this rabbit hole a couple of times. The initializer for a 
variable is built differently based on the context. For instance, range based 
for loops defer building the initializer (for whatever reason idk,) but it all 
happens in the call to VisitVarDecl which builds the new DecompositionDecl. I 
do not know what it would take to rip all of that out of there.

See 
https://github.com/ricejasonf/llvm-project/blob/ricejasonf/p1061r9/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L1193
and
https://github.com/ricejasonf/llvm-project/blob/ricejasonf/p1061r9/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp#L1224

The above attempt was building the initializer just to get the number of 
bindings and discarding it which felt like a waste.

Forgive me, IF I am misunderstanding what you are saying here. :rofl: 

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -951,28 +959,124 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator 
&D,
   return New;
 }
 
+// CheckBindingsCount
+//  - Checks the arity of the structured bindings
+//  - Creates the resolved pack expr if there is
+//one
+static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
+   QualType DecompType,
+   ArrayRef Bindings,
+   unsigned MemberCount) {
+  auto BindingWithPackItr =
+  std::find_if(Bindings.begin(), Bindings.end(),
+   [](BindingDecl *D) -> bool { return D->isParameterPack(); 
});
+  bool HasPack = BindingWithPackItr != Bindings.end();
+  bool IsValid;
+  if (!HasPack) {
+IsValid = Bindings.size() == MemberCount;
+  } else {
+// there may not be more members than non-pack bindings
+IsValid = MemberCount >= Bindings.size() - 1;
+  }
+
+  if (IsValid && HasPack) {
+// create the pack expr and assign it to the binding
+unsigned PackSize = MemberCount - Bindings.size() + 1;
+QualType PackType = S.Context.getPackExpansionType(
+S.Context.DependentTy, std::nullopt, /*ExpectsPackInType=*/false);
+BindingDecl *BD = (*BindingWithPackItr);
+BD->setBinding(PackType,
+   ResolvedUnexpandedPackExpr::Create(
+   S.Context, DD->getBeginLoc(), DecompType, PackSize));

ricejasonf wrote:

>(During the initial parse, we should treat a structured binding declaration 
>with a binding pack as being dependent, just like we treat a case where the 
>initializer has an unexpanded pack as dependent.)

The BindingDecls are still created even when the initializer is dependent, and 
still the init expression is created after the DecompositionDecl even in 
template instantiation. (I remember trying this, but template instantation does 
not like instantiation of the same local decl twice.)

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -15991,6 +15998,24 @@ 
TreeTransform::TransformFunctionParmPackExpr(FunctionParmPackExpr *E) {
   return E;
 }
 
+template 
+ExprResult TreeTransform::TransformResolvedUnexpandedPackExpr(
+ResolvedUnexpandedPackExpr *E) {
+  bool ArgumentChanged = false;
+  SmallVector NewExprs;
+  if (TransformExprs(E->getExprs().begin(), E->getNumExprs(),
+ /*IsCall=*/false, NewExprs, &ArgumentChanged))
+return ExprError();
+
+  if (!AlwaysRebuild() && !ArgumentChanged)
+return E;
+
+  // NOTE: The type is just a superficial PackExpansionType
+  //   that needs no substitution.

ricejasonf wrote:

I am not sure I understand what you mean here. It is still an unexpanded pack 
that were are just rebuilding.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -1166,26 +1166,54 @@ 
TemplateDeclInstantiator::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
 
 Decl *TemplateDeclInstantiator::VisitBindingDecl(BindingDecl *D) {
   auto *NewBD = BindingDecl::Create(SemaRef.Context, Owner, D->getLocation(),
-D->getIdentifier());
+D->getIdentifier(), D->getType());
   NewBD->setReferenced(D->isReferenced());
   SemaRef.CurrentInstantiationScope->InstantiatedLocal(D, NewBD);
+
   return NewBD;
 }
 
 Decl *TemplateDeclInstantiator::VisitDecompositionDecl(DecompositionDecl *D) {
   // Transform the bindings first.
+  // The transformed DD will have all of the concrete BindingDecls.
   SmallVector NewBindings;
-  for (auto *OldBD : D->bindings())
+  ResolvedUnexpandedPackExpr *OldResolvedPack = nullptr;
+  for (auto *OldBD : D->bindings()) {
+Expr *BindingExpr = OldBD->getBinding();
+if (auto *RP = 
dyn_cast_if_present(BindingExpr))
+  OldResolvedPack = RP;

ricejasonf wrote:

The `isa` call would fail on a null type so I just left a 
comment.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -50,17 +50,29 @@ class CollectUnexpandedParameterPacksVisitor
 auto *FTD = FD ? FD->getDescribedFunctionTemplate() : nullptr;
 if (FTD && FTD->getTemplateParameters()->getDepth() >= DepthLimit)
   return;
-  } else if (getDepthAndIndex(ND).first >= DepthLimit)
+  } else if (auto *BD = dyn_cast(ND)) {
+Expr *E = BD->getBinding();
+if (auto *RP = dyn_cast_if_present(E)) {
+  addUnexpanded(RP);
+  return;
+}

ricejasonf wrote:

I changed it to `cast_if_present` since a dependent decomposition leaves the 
binding expr as nullptr.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -1166,26 +1166,54 @@ 
TemplateDeclInstantiator::VisitTypeAliasTemplateDecl(TypeAliasTemplateDecl *D) {
 
 Decl *TemplateDeclInstantiator::VisitBindingDecl(BindingDecl *D) {
   auto *NewBD = BindingDecl::Create(SemaRef.Context, Owner, D->getLocation(),
-D->getIdentifier());
+D->getIdentifier(), D->getType());
   NewBD->setReferenced(D->isReferenced());
   SemaRef.CurrentInstantiationScope->InstantiatedLocal(D, NewBD);
+
   return NewBD;
 }
 
 Decl *TemplateDeclInstantiator::VisitDecompositionDecl(DecompositionDecl *D) {
   // Transform the bindings first.
+  // The transformed DD will have all of the concrete BindingDecls.
   SmallVector NewBindings;
-  for (auto *OldBD : D->bindings())
+  ResolvedUnexpandedPackExpr *OldResolvedPack = nullptr;
+  for (auto *OldBD : D->bindings()) {
+Expr *BindingExpr = OldBD->getBinding();
+if (auto *RP = 
dyn_cast_if_present(BindingExpr))
+  OldResolvedPack = RP;
 NewBindings.push_back(cast(VisitBindingDecl(OldBD)));
+  }
   ArrayRef NewBindingArray = NewBindings;
 
-  auto *NewDD = cast_or_null(
+  auto *NewDD = cast_if_present(
   VisitVarDecl(D, /*InstantiatingVarTemplate=*/false, &NewBindingArray));
 
   if (!NewDD || NewDD->isInvalidDecl())
 for (auto *NewBD : NewBindings)
   NewBD->setInvalidDecl();
 
+  if (OldResolvedPack) {
+// Mark the holding vars (if any) in the pack as instantiated since
+// they are created implicitly.
+auto Bindings = NewDD->bindings();
+auto BPack = std::find_if(
+Bindings.begin(), Bindings.end(),
+[](BindingDecl *D) -> bool { return D->isParameterPack(); });
+auto *NewResolvedPack =
+cast((*BPack)->getBinding());
+auto OldExprs = OldResolvedPack->getExprs();
+auto NewExprs = NewResolvedPack->getExprs();

ricejasonf wrote:

added

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -757,23 +775,40 @@ bool Sema::CheckParameterPacksForExpansion(
   bool HaveFirstPack = false;
   std::optional NumPartialExpansions;
   SourceLocation PartiallySubstitutedPackLoc;
+  typedef LocalInstantiationScope::DeclArgumentPack DeclArgumentPack;
 
   for (UnexpandedParameterPack ParmPack : Unexpanded) {
 // Compute the depth and index for this parameter pack.
 unsigned Depth = 0, Index = 0;
 IdentifierInfo *Name;
 bool IsVarDeclPack = false;
+ResolvedUnexpandedPackExpr *ResolvedPack = nullptr;
 
 if (const TemplateTypeParmType *TTP =
 ParmPack.first.dyn_cast()) {
   Depth = TTP->getDepth();
   Index = TTP->getIndex();
   Name = TTP->getIdentifier();
+} else if (auto *RP =
+   ParmPack.first.dyn_cast()) {
+  ResolvedPack = RP;
 } else {
   NamedDecl *ND = cast(ParmPack.first);
   if (isa(ND))
 IsVarDeclPack = true;
-  else
+  else if (isa(ND)) {
+// Find the instantiated BindingDecl and check it for a resolved pack.
+llvm::PointerUnion *Instantiation =
+CurrentInstantiationScope->findInstantiationOf(ND);
+Decl *B = cast(*Instantiation);
+Expr *BindingExpr = cast(B)->getBinding();
+ResolvedPack =
+dyn_cast_if_present(BindingExpr);
+if (!ResolvedPack) {
+  ShouldExpand = false;
+  continue;
+}

ricejasonf wrote:

This case is definitely reached. (I tried it.) I think I could make this 
`cast_if_present`.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -951,28 +959,124 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator 
&D,
   return New;
 }
 
+// CheckBindingsCount
+//  - Checks the arity of the structured bindings
+//  - Creates the resolved pack expr if there is
+//one
+static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
+   QualType DecompType,
+   ArrayRef Bindings,
+   unsigned MemberCount) {
+  auto BindingWithPackItr =
+  std::find_if(Bindings.begin(), Bindings.end(),
+   [](BindingDecl *D) -> bool { return D->isParameterPack(); 
});
+  bool HasPack = BindingWithPackItr != Bindings.end();
+  bool IsValid;
+  if (!HasPack) {
+IsValid = Bindings.size() == MemberCount;
+  } else {
+// there may not be more members than non-pack bindings
+IsValid = MemberCount >= Bindings.size() - 1;
+  }
+
+  if (IsValid && HasPack) {
+// create the pack expr and assign it to the binding
+unsigned PackSize = MemberCount - Bindings.size() + 1;
+QualType PackType = S.Context.getPackExpansionType(
+S.Context.DependentTy, std::nullopt, /*ExpectsPackInType=*/false);
+BindingDecl *BD = (*BindingWithPackItr);
+BD->setBinding(PackType,
+   ResolvedUnexpandedPackExpr::Create(
+   S.Context, DD->getBeginLoc(), DecompType, PackSize));

ricejasonf wrote:

Here is a PR to this branch so you can see what would have to change to remove 
BindingInitWalker like that. Let me know, and I will add the commit to this 
branch.
https://github.com/ricejasonf/llvm-project/pull/1/files

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -1523,9 +1570,25 @@ void 
Sema::CheckCompleteDecompositionDeclaration(DecompositionDecl *DD) {
   // If the type of the decomposition is dependent, then so is the type of
   // each binding.
   if (DecompType->isDependentType()) {
-for (auto *B : DD->bindings())
-  B->setType(Context.DependentTy);
+for (auto *B : DD->bindings()) {
+  // Do not overwrite any pack type.
+  if (B->getType().isNull())
+B->setType(Context.DependentTy);
+}

ricejasonf wrote:

This was here before. After processing the intializer the BindingDecls should 
all have types set except in the case when the initializer is dependent. In 
that case all the types are still Null so here we set them to dependent. My 
change is just preventing it from overwriting the PackExpansionType if there is 
one.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -3423,6 +3425,13 @@ VarDecl *BindingDecl::getHoldingVar() const {
   return VD;
 }
 
+llvm::ArrayRef BindingDecl::getBindingPackExprs() const {
+  if (!Binding)
+return {};

ricejasonf wrote:

Added. The assert was being triggered in one spot that I realized was 
unnecessary (when I tried removing the whole block of code that was going back 
and adding types to the DeclRefExprs.)

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -1523,9 +1570,25 @@ void 
Sema::CheckCompleteDecompositionDeclaration(DecompositionDecl *DD) {
   // If the type of the decomposition is dependent, then so is the type of
   // each binding.
   if (DecompType->isDependentType()) {
-for (auto *B : DD->bindings())
-  B->setType(Context.DependentTy);
+for (auto *B : DD->bindings()) {
+  // Do not overwrite any pack type.
+  if (B->getType().isNull())
+B->setType(Context.DependentTy);
+}

ricejasonf wrote:

The comment I updated is above this.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -4213,8 +4226,35 @@ class DecompositionDecl final
   static DecompositionDecl *CreateDeserialized(ASTContext &C, GlobalDeclID ID,
unsigned NumBindings);
 
-  ArrayRef bindings() const {
-return llvm::ArrayRef(getTrailingObjects(), NumBindings);
+  // Provide the range of bindings which may have a nested pack.
+  llvm::ArrayRef bindings() const {
+return {getTrailingObjects(), NumBindings};
+  }
+
+  // Provide a flattened range to visit each binding.
+  auto flat_bindings() const {
+llvm::ArrayRef Bindings = bindings();
+llvm::ArrayRef PackExprs;
+
+// Split the bindings into subranges split by the pack.
+auto S1 = Bindings.take_until(
+[](BindingDecl *BD) { return BD->isParameterPack(); });
+
+Bindings = Bindings.drop_front(S1.size());
+if (!Bindings.empty()) {
+  PackExprs = Bindings.front()->getBindingPackExprs();
+  Bindings = Bindings.drop_front();
+}
+
+auto S2 = llvm::map_range(PackExprs, [](Expr *E) {
+  auto *DRE = cast(E);
+  return cast(DRE->getDecl());
+});
+
+// llvm::concat must take temporaries or it will capture
+// references.

ricejasonf wrote:

Ah, right. I just removed the comment.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -5321,6 +5321,59 @@ class BuiltinBitCastExpr final
   }
 };
 
+// Represents an unexpanded pack where the list of expressions are
+// known. These are used when structured bindings introduce a pack.
+class ResolvedUnexpandedPackExpr final
+: public Expr,
+  private llvm::TrailingObjects {

ricejasonf wrote:

Probably. That would simplify the DeclRefExpr stuff and a few other things. 
`BindingPackExpr`?

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -1099,6 +1099,13 @@ def err_lambda_capture_misplaced_ellipsis : Error<
   "the name of the capture">;
 def err_lambda_capture_multiple_ellipses : Error<
   "multiple ellipses in pack capture">;
+def err_binding_multiple_ellipses : Error<
+  "multiple packs in structured binding declaration">;
+def note_previous_ellipsis : Note<
+  "previous binding pack specified here">;
+def ext_cxx_binding_pack : ExtWarn<

ricejasonf wrote:

What warning are we wanting to display in C++26 mode? The 
`ext_cxx_binding_pack` was added for when we are **not** in C++26 mode right?

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++26 %s -verify

ricejasonf wrote:

I added these to the bottom of this file.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

ricejasonf wrote:

>@ricejasonf let us know if you want us to merge on your behalf

@cor3ntin When you think its ready, yes please. I think it requires write 
access. I fixed the conflict in the ReleaseNotes through Github's UI so that 
only my feature is under the C++2c features added. I assume other ones were 
removed as they are for the "previous release".

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

ricejasonf wrote:

@cor3ntin 

> Now that the Clang 20 branch has happened, I think we should go ahead with 
> that PR

Does this mean we should adjust the cxx_status page to not say Clang 20? I also 
see there is a conflict with the ReleaseNotes.md where it appears all of the 
C++2c items no longer exist. (I guess that is because it is for the next 
release.)

> It would be great to explore merging ResolvedUnexpandedPackExpr and 
> FunctionParmPackExpr in a separate PR. Is that something you would be 
> interested in exploring?

I can look into this. I will consider `SubstNonTypeTemplateParmPackExpr` as 
well although I am not sure how special that one is.

>Thank you Jason for contributing this feature and for being reactive on this 
>rather involved review :)

Likewise and thank you and the others for your contributions and diligence 
while reviewing this!

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

ricejasonf wrote:

That crash looks like a result of my call to setDecomposedDecl(nullptr) in
the BindingsInitWalker stuff. I don't remember why I don't set that. I will
check it out when I try to clean that up.

> But that also makes it confusing as to this implementation - I suggest
avoiding unrelated changes for better clarity and safety. WDYT?
The change is related. We do not set `Name` for
ResolvedUnexpandedPackExprs. It was previously assuming the remaining case
must be NamedDecl.

On Thu, Jan 2, 2025 at 10:06 PM Younan Zhang ***@***.***>
wrote:

> ***@***. commented on this pull request.
>
> Here is an interesting case involving lambda captures:
>
> https://godbolt.org/z/Y7EhE7Gvq
>
> (Everything would become scary when lambda comes into play)
>
> —
> Reply to this email directly, view it on GitHub
> ,
> or unsubscribe
> 
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>


https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -1508,23 +1612,52 @@ static bool checkMemberDecomposition(Sema &S, 
ArrayRef Bindings,
 Qualifiers Q = DecompType.getQualifiers();
 if (FD->isMutable())
   Q.removeConst();
-B->setBinding(S.BuildQualifiedType(FD->getType(), Loc, Q), E.get());
+Walker.commitAndAdvance(S.BuildQualifiedType(FD->getType(), Loc, Q),
+E.get());
   }
 
-  if (I != Bindings.size())
-return DiagnoseBadNumberOfBindings();
-
   return false;
 }
 
+unsigned Sema::GetDecompositionElementCount(QualType DecompType) {
+  assert(!DecompType->isDependentType() && "expecting non-dependent type");
+  SourceLocation Loc = SourceLocation(); // FIXME
+  DecompType = DecompType.getNonReferenceType();
+  if (auto *CAT = Context.getAsConstantArrayType(DecompType))
+return CAT->getSize().getLimitedValue(UINT_MAX);
+  if (auto *VT = DecompType->getAs())
+return VT->getNumElements();
+  if (auto *CT = DecompType->getAs())
+return 2;
+  llvm::APSInt TupleSize(32);
+  if (IsTupleLike TL = isTupleLike(*this, Loc, DecompType, TupleSize);
+  TL == IsTupleLike::TupleLike)
+return (unsigned)TupleSize.getLimitedValue(UINT_MAX);
+
+  if (CXXRecordDecl *RD = DecompType->getAsCXXRecordDecl();
+  RD && !RD->isUnion()) {
+CXXCastPath BasePath;
+DeclAccessPair BasePair =
+findDecomposableBaseClass(*this, Loc, RD, BasePath);
+RD = cast_or_null(BasePair.getDecl());
+if (RD)
+  return llvm::count_if(
+  RD->fields(), [](FieldDecl *FD) { return !FD->isUnnamedBitField(); 
});
+  }
+
+  llvm_unreachable("unknown type for decomposition");
+}
+
 void Sema::CheckCompleteDecompositionDeclaration(DecompositionDecl *DD) {
   QualType DecompType = DD->getType();
 
   // If the type of the decomposition is dependent, then so is the type of
   // each binding.
   if (DecompType->isDependentType()) {
-for (auto *B : DD->bindings())
-  B->setType(Context.DependentTy);
+for (auto *B : DD->bindings()) {
+  if (B->getType().isNull())
+B->setType(Context.DependentTy);
+}

ricejasonf wrote:

I do not recall where exactly, but somewhere in the attempt to expand packs, it 
did not like the types being null. The types are usually set when the init 
expression is instantiated.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -50,16 +51,28 @@ class CollectUnexpandedParameterPacksVisitor
 auto *FTD = FD ? FD->getDescribedFunctionTemplate() : nullptr;
 if (FTD && FTD->getTemplateParameters()->getDepth() >= DepthLimit)
   return;
-  } else if (getDepthAndIndex(ND).first >= DepthLimit)
+  } else if (ND->isTemplateParameterPack() &&

ricejasonf wrote:

I was adding the case for BindingDecl. I see template parameters could be kept 
as the last case. I will try that.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -951,28 +959,130 @@ Sema::ActOnDecompositionDeclarator(Scope *S, Declarator 
&D,
   return New;
 }
 
+namespace {
+// CheckBindingsCount
+//  - Checks the arity of the structured bindings
+//  - Creates the resolved pack expr if there is
+//one
+bool CheckBindingsCount(Sema &S, DecompositionDecl *DD, QualType DecompType,
+ArrayRef Bindings,
+unsigned MemberCount) {
+  auto BindingWithPackItr =
+  std::find_if(Bindings.begin(), Bindings.end(),
+   [](BindingDecl *D) -> bool { return D->isParameterPack(); 
});
+  bool HasPack = BindingWithPackItr != Bindings.end();
+  bool IsValid;
+  if (!HasPack) {
+IsValid = Bindings.size() == MemberCount;
+  } else {
+// there may not be more members than non-pack bindings
+IsValid = MemberCount >= Bindings.size() - 1;
+  }
+
+  if (IsValid && HasPack) {
+TemplateTypeParmDecl *DummyTemplateParam = TemplateTypeParmDecl::Create(
+S.Context, S.Context.getTranslationUnitDecl(),
+/*KeyLoc*/ SourceLocation(), /*NameLoc*/ SourceLocation(),
+/*TemplateDepth*/ 0, /*AutoParameterPosition*/ 0,
+/*Identifier*/ nullptr, false, /*IsParameterPack*/ true);

ricejasonf wrote:

The first line of `getPackExpansionType` has an assert on 
`Pattern->containsUnexpandedParameterPack()`. Now that I am looking at it, 
there is a `ExpectsPackInType` parameter. Maybe that was not there before or I 
just missed it. I will try that.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -422,8 +445,8 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
 if (const TemplateTypeParmType *TTP
   = Unexpanded[I].first.dyn_cast())
   Name = TTP->getIdentifier();
-else
-  Name = cast(Unexpanded[I].first)->getIdentifier();
+else if (NamedDecl *ND = Unexpanded[I].first.dyn_cast())
+  Name = ND->getIdentifier();

ricejasonf wrote:

The change is related. We do not set `Name` for ResolvedUnexpandedPackExprs. It 
was previously assuming the remaining case must be NamedDecl. (from my email)

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -422,8 +445,8 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
 if (const TemplateTypeParmType *TTP
   = Unexpanded[I].first.dyn_cast())
   Name = TTP->getIdentifier();
-else
-  Name = cast(Unexpanded[I].first)->getIdentifier();
+else if (NamedDecl *ND = Unexpanded[I].first.dyn_cast())
+  Name = ND->getIdentifier();

ricejasonf wrote:

It is possible there is a completely different approach that uses BindingDecl 
more directly as the parameter pack. I think it would still run into issues 
with BindingDecl not being a VarDecl so the ResolvedUnexpandedPackExpr worked 
out, but it was part of a more general approach to dealing with packs.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/121417

>From 3c81c5bd989f26331917f1401becc1cfa7f4a454 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Thu, 22 Jul 2021 16:46:33 -0700
Subject: [PATCH 1/6] [Clang][P1061] stuctured binding packs

---
 clang/include/clang/AST/Decl.h|   8 +-
 clang/include/clang/AST/DeclCXX.h |  22 +-
 clang/include/clang/AST/ExprCXX.h |  48 
 clang/include/clang/AST/RecursiveASTVisitor.h |   1 +
 .../clang/Basic/DiagnosticParseKinds.td   |   5 +
 .../clang/Basic/DiagnosticSemaKinds.td|   3 +
 clang/include/clang/Basic/StmtNodes.td|   1 +
 clang/include/clang/Sema/DeclSpec.h   |   1 +
 clang/include/clang/Sema/Sema.h   |   4 +-
 .../include/clang/Serialization/ASTBitCodes.h |   1 +
 clang/lib/AST/ASTContext.cpp  |  14 +-
 clang/lib/AST/ASTImporter.cpp |   2 +-
 clang/lib/AST/Decl.cpp|  11 +-
 clang/lib/AST/DeclBase.cpp|   6 +-
 clang/lib/AST/DeclCXX.cpp |  60 -
 clang/lib/AST/Expr.cpp|   5 +
 clang/lib/AST/ExprCXX.cpp |  48 
 clang/lib/AST/ExprClassification.cpp  |   7 +
 clang/lib/AST/ExprConstant.cpp|   5 +-
 clang/lib/AST/ItaniumMangle.cpp   |   2 +-
 clang/lib/AST/StmtPrinter.cpp |  11 +
 clang/lib/AST/StmtProfile.cpp |   4 +
 clang/lib/CodeGen/CGDebugInfo.cpp |   4 +-
 clang/lib/CodeGen/CGDecl.cpp  |   5 +-
 clang/lib/CodeGen/CodeGenModule.cpp   |   4 +-
 clang/lib/Parse/ParseDecl.cpp |  28 ++-
 clang/lib/Sema/SemaDecl.cpp   |  10 +
 clang/lib/Sema/SemaDeclCXX.cpp| 209 ++
 clang/lib/Sema/SemaExceptionSpec.cpp  |   7 +-
 clang/lib/Sema/SemaLambda.cpp |   1 +
 clang/lib/Sema/SemaStmt.cpp   |  14 +-
 clang/lib/Sema/SemaTemplate.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  40 +++-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  32 ++-
 clang/lib/Sema/SemaTemplateVariadic.cpp   |  73 +-
 clang/lib/Sema/TreeTransform.h|   7 +
 clang/lib/Serialization/ASTReaderStmt.cpp |  11 +
 clang/lib/Serialization/ASTWriter.cpp |   1 +
 clang/lib/Serialization/ASTWriterStmt.cpp |  11 +
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp  |   1 +
 clang/test/Parser/cxx2c-binding-pack.cpp  |   7 +
 .../cxx2c-binding-pack-nontemplate.cpp|  12 +
 clang/test/SemaCXX/cxx2c-binding-pack.cpp |  82 +++
 clang/test/SemaCXX/typo-correction-crash.cpp  |   3 +-
 clang/tools/libclang/CXCursor.cpp |   1 +
 45 files changed, 728 insertions(+), 111 deletions(-)
 create mode 100644 clang/test/Parser/cxx2c-binding-pack.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack.cpp

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..bdf6c81732d0bc 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -698,6 +698,10 @@ class ValueDecl : public NamedDecl {
 return const_cast(this)->getPotentiallyDecomposedVarDecl();
   }
 
+  /// Determine whether this value is actually a function parameter pack,
+  /// init-capture pack, or structured binding pack
+  bool isParameterPack() const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
@@ -1527,10 +1531,6 @@ class VarDecl : public DeclaratorDecl, public 
Redeclarable {
 NonParmVarDeclBits.IsInitCapture = IC;
   }
 
-  /// Determine whether this variable is actually a function parameter pack or
-  /// init-capture pack.
-  bool isParameterPack() const;
-
   /// Whether this local extern variable declaration's previous declaration
   /// was declared in the same block scope. Only correct in C++.
   bool isPreviousDeclInSameBlockScope() const {
diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index c232556edeff70..12002db17fb3ad 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4131,8 +4131,9 @@ class BindingDecl : public ValueDecl {
   /// binding).
   Expr *Binding = nullptr;
 
-  BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id)
-  : ValueDecl(Decl::Binding, DC, IdLoc, Id, QualType()) {}
+  BindingDecl(DeclContext *DC, SourceLocation IdLoc, IdentifierInfo *Id,
+  QualType T)
+  : ValueDecl(Decl::Binding, DC, IdLoc, Id, T) {}
 
   void anchor() override;
 
@@ -4140,7 +4141,8 @@ class BindingDecl : public ValueDecl {
   friend class ASTDeclReader;
 
   static BindingDecl *Create(ASTContext

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

ricejasonf wrote:

@erichkeane, thanks for the approval. Does that mean I should stop making 
changes to this PR? I was messing with a more rangey interface for 
`flat_bindings`, but for some reason the `llvm::concat` range was referring to 
uninitialized memory when used as a temporary in a for loop (which even 
pre-C++23 I could not see why this was the case). I am not asking anyone to 
debug this, but I just wanted to know if this was the kind of "iterator" thing 
that you were requesting.
```cpp
  // Provide a flattened range to visit each binding.
  auto flat_bindings() const {
llvm::ArrayRef Bindings = bindings();
llvm::ArrayRef PackExprs;

// Split the bindings into subranges split by the pack.
auto S1 = Bindings.take_until([](BindingDecl* BD) {
  return BD->isParameterPack();
});  

Bindings = Bindings.drop_front(S1.size());
if (!Bindings.empty()) {
  PackExprs = Bindings.front()->getBindingPackExprs();
  Bindings = Bindings.drop_front();
}

auto S2 = llvm::map_range(PackExprs, [](Expr* E) { 
  auto *DRE = cast(E);
  return cast(DRE->getDecl());
});  

return llvm::concat(S1, S2, Bindings);
  }
```

I appreciate the people who have reviewed this PR as it did come with some of 
its own legacy and cruft.

Let me know if you want me to address any of the remaining concerns or not.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -1508,23 +1612,52 @@ static bool checkMemberDecomposition(Sema &S, 
ArrayRef Bindings,
 Qualifiers Q = DecompType.getQualifiers();
 if (FD->isMutable())
   Q.removeConst();
-B->setBinding(S.BuildQualifiedType(FD->getType(), Loc, Q), E.get());
+Walker.commitAndAdvance(S.BuildQualifiedType(FD->getType(), Loc, Q),
+E.get());
   }
 
-  if (I != Bindings.size())
-return DiagnoseBadNumberOfBindings();
-
   return false;
 }
 
+unsigned Sema::GetDecompositionElementCount(QualType DecompType) {
+  assert(!DecompType->isDependentType() && "expecting non-dependent type");
+  SourceLocation Loc = SourceLocation(); // FIXME
+  DecompType = DecompType.getNonReferenceType();
+  if (auto *CAT = Context.getAsConstantArrayType(DecompType))
+return CAT->getSize().getLimitedValue(UINT_MAX);
+  if (auto *VT = DecompType->getAs())
+return VT->getNumElements();
+  if (auto *CT = DecompType->getAs())
+return 2;
+  llvm::APSInt TupleSize(32);
+  if (IsTupleLike TL = isTupleLike(*this, Loc, DecompType, TupleSize);
+  TL == IsTupleLike::TupleLike)
+return (unsigned)TupleSize.getLimitedValue(UINT_MAX);
+
+  if (CXXRecordDecl *RD = DecompType->getAsCXXRecordDecl();
+  RD && !RD->isUnion()) {
+CXXCastPath BasePath;
+DeclAccessPair BasePair =
+findDecomposableBaseClass(*this, Loc, RD, BasePath);
+RD = cast_or_null(BasePair.getDecl());
+if (RD)
+  return llvm::count_if(
+  RD->fields(), [](FieldDecl *FD) { return !FD->isUnnamedBitField(); 
});
+  }
+
+  llvm_unreachable("unknown type for decomposition");
+}
+
 void Sema::CheckCompleteDecompositionDeclaration(DecompositionDecl *DD) {
   QualType DecompType = DD->getType();
 
   // If the type of the decomposition is dependent, then so is the type of
   // each binding.
   if (DecompType->isDependentType()) {
-for (auto *B : DD->bindings())
-  B->setType(Context.DependentTy);
+for (auto *B : DD->bindings()) {
+  if (B->getType().isNull())
+B->setType(Context.DependentTy);
+}

ricejasonf wrote:

Looking at this again, I see that I was avoiding replacing the 
PackExpansionType here.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -12726,11 +12726,15 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
 
   // Likewise, variables with tuple-like bindings are required if their
   // bindings have side-effects.
-  if (const auto *DD = dyn_cast(VD))
-for (const auto *BD : DD->bindings())
-  if (const auto *BindingVD = BD->getHoldingVar())
-if (DeclMustBeEmitted(BindingVD))
-  return true;
+  if (const auto *DD = dyn_cast(VD)) {
+bool BindingResult = false;
+DD->VisitHoldingVars([&](VarDecl *BindingVD) {

ricejasonf wrote:

I replaced the Visit functions with `flat_bindings` and reverted the places 
where I was using them. I will look to see if there is some existing iterator 
that you are referring to if it would be simple than `flat_binding_iterator` 
(which had some things I had to fix.)

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -1965,3 +1965,52 @@ CXXFoldExpr::CXXFoldExpr(QualType T, 
UnresolvedLookupExpr *Callee,
   SubExprs[SubExpr::RHS] = RHS;
   setDependence(computeDependence(this));
 }
+
+ResolvedUnexpandedPackExpr::ResolvedUnexpandedPackExpr(SourceLocation BL,
+   QualType QT,
+   unsigned NumExprs)
+: Expr(ResolvedUnexpandedPackExprClass, QT, VK_PRValue, OK_Ordinary),
+  BeginLoc(BL), NumExprs(NumExprs) {
+  // C++ [temp.dep.expr]p3
+  // An id-expression is type-dependent if it is
+  //- associated by name lookup with a pack
+  setDependence(ExprDependence::TypeValueInstantiation |
+ExprDependence::UnexpandedPack);
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::CreateDeserialized(ASTContext &Ctx,
+   unsigned NumExprs) {
+  void *Mem = Ctx.Allocate(totalSizeToAlloc(NumExprs),
+   alignof(ResolvedUnexpandedPackExpr));
+  return new (Mem)
+  ResolvedUnexpandedPackExpr(SourceLocation(), QualType(), NumExprs);
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
+   QualType T, unsigned NumExprs) {
+  void *Mem = Ctx.Allocate(totalSizeToAlloc(NumExprs),
+   alignof(ResolvedUnexpandedPackExpr));
+  ResolvedUnexpandedPackExpr *New =
+  new (Mem) ResolvedUnexpandedPackExpr(BL, T, NumExprs);
+
+  auto Exprs = llvm::MutableArrayRef(New->getExprs(), New->getNumExprs());
+  std::fill(Exprs.begin(), Exprs.end(), nullptr);
+
+  return New;
+}
+
+ResolvedUnexpandedPackExpr *
+ResolvedUnexpandedPackExpr::Create(ASTContext &Ctx, SourceLocation BL,
+   QualType T, ArrayRef Exprs) {
+  auto *New = Create(Ctx, BL, T, Exprs.size());
+  std::copy(Exprs.begin(), Exprs.end(), New->getExprs());

ricejasonf wrote:

I suppose it could. The other Create function it is calling here is using 
`std::fill` which could probably be `std::uninitialized_fill`.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf edited 
https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf created 
https://github.com/llvm/llvm-project/pull/121417

This is an implementation of P1061 Structure Bindings Introduce a Pack without  
the ability to use packs outside of templates. There is a couple of ways the 
AST could have been sliced so let me know what you think. The only part of this 
change that I am unsure of is the serialization/deserialization stuff. I 
followed the implementation of other Exprs, but I do not really know how it is 
tested. Thank you for your time considering this.

>From 3c81c5bd989f26331917f1401becc1cfa7f4a454 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Thu, 22 Jul 2021 16:46:33 -0700
Subject: [PATCH] [Clang][P1061] stuctured binding packs

---
 clang/include/clang/AST/Decl.h|   8 +-
 clang/include/clang/AST/DeclCXX.h |  22 +-
 clang/include/clang/AST/ExprCXX.h |  48 
 clang/include/clang/AST/RecursiveASTVisitor.h |   1 +
 .../clang/Basic/DiagnosticParseKinds.td   |   5 +
 .../clang/Basic/DiagnosticSemaKinds.td|   3 +
 clang/include/clang/Basic/StmtNodes.td|   1 +
 clang/include/clang/Sema/DeclSpec.h   |   1 +
 clang/include/clang/Sema/Sema.h   |   4 +-
 .../include/clang/Serialization/ASTBitCodes.h |   1 +
 clang/lib/AST/ASTContext.cpp  |  14 +-
 clang/lib/AST/ASTImporter.cpp |   2 +-
 clang/lib/AST/Decl.cpp|  11 +-
 clang/lib/AST/DeclBase.cpp|   6 +-
 clang/lib/AST/DeclCXX.cpp |  60 -
 clang/lib/AST/Expr.cpp|   5 +
 clang/lib/AST/ExprCXX.cpp |  48 
 clang/lib/AST/ExprClassification.cpp  |   7 +
 clang/lib/AST/ExprConstant.cpp|   5 +-
 clang/lib/AST/ItaniumMangle.cpp   |   2 +-
 clang/lib/AST/StmtPrinter.cpp |  11 +
 clang/lib/AST/StmtProfile.cpp |   4 +
 clang/lib/CodeGen/CGDebugInfo.cpp |   4 +-
 clang/lib/CodeGen/CGDecl.cpp  |   5 +-
 clang/lib/CodeGen/CodeGenModule.cpp   |   4 +-
 clang/lib/Parse/ParseDecl.cpp |  28 ++-
 clang/lib/Sema/SemaDecl.cpp   |  10 +
 clang/lib/Sema/SemaDeclCXX.cpp| 209 ++
 clang/lib/Sema/SemaExceptionSpec.cpp  |   7 +-
 clang/lib/Sema/SemaLambda.cpp |   1 +
 clang/lib/Sema/SemaStmt.cpp   |  14 +-
 clang/lib/Sema/SemaTemplate.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp|  40 +++-
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  32 ++-
 clang/lib/Sema/SemaTemplateVariadic.cpp   |  73 +-
 clang/lib/Sema/TreeTransform.h|   7 +
 clang/lib/Serialization/ASTReaderStmt.cpp |  11 +
 clang/lib/Serialization/ASTWriter.cpp |   1 +
 clang/lib/Serialization/ASTWriterStmt.cpp |  11 +
 clang/lib/StaticAnalyzer/Core/ExprEngine.cpp  |   1 +
 clang/test/Parser/cxx2c-binding-pack.cpp  |   7 +
 .../cxx2c-binding-pack-nontemplate.cpp|  12 +
 clang/test/SemaCXX/cxx2c-binding-pack.cpp |  82 +++
 clang/test/SemaCXX/typo-correction-crash.cpp  |   3 +-
 clang/tools/libclang/CXCursor.cpp |   1 +
 45 files changed, 728 insertions(+), 111 deletions(-)
 create mode 100644 clang/test/Parser/cxx2c-binding-pack.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
 create mode 100644 clang/test/SemaCXX/cxx2c-binding-pack.cpp

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 67ee0bb412692a..bdf6c81732d0bc 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -698,6 +698,10 @@ class ValueDecl : public NamedDecl {
 return const_cast(this)->getPotentiallyDecomposedVarDecl();
   }
 
+  /// Determine whether this value is actually a function parameter pack,
+  /// init-capture pack, or structured binding pack
+  bool isParameterPack() const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
   static bool classofKind(Kind K) { return K >= firstValue && K <= lastValue; }
@@ -1527,10 +1531,6 @@ class VarDecl : public DeclaratorDecl, public 
Redeclarable {
 NonParmVarDeclBits.IsInitCapture = IC;
   }
 
-  /// Determine whether this variable is actually a function parameter pack or
-  /// init-capture pack.
-  bool isParameterPack() const;
-
   /// Whether this local extern variable declaration's previous declaration
   /// was declared in the same block scope. Only correct in C++.
   bool isPreviousDeclInSameBlockScope() const {
diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index c232556edeff70..12002db17fb3ad 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4131,8 +4131,9 @@ class BindingDecl : public ValueDecl {
   /// binding).
   Expr *Binding = nullptr;
 
-  BindingDecl(Dec

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -4219,6 +4220,13 @@ class DecompositionDecl final
 
   void printName(raw_ostream &OS, const PrintingPolicy &Policy) const override;
 
+  /// Visit the variables (if any) that hold the values of evaluating the
+  /// binding. Only present for user-defined bindings for tuple-like types.
+  void VisitHoldingVars(llvm::function_ref F) const;
+
+  // Visit the concrete bindings. (workaround)

ricejasonf wrote:

I'll just remove the "(workaround)", but if there is a better range based 
solution that might be preferred.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -422,8 +445,8 @@ Sema::DiagnoseUnexpandedParameterPacks(SourceLocation Loc,
 if (const TemplateTypeParmType *TTP
   = Unexpanded[I].first.dyn_cast())
   Name = TTP->getIdentifier();
-else
-  Name = cast(Unexpanded[I].first)->getIdentifier();
+else if (NamedDecl *ND = Unexpanded[I].first.dyn_cast())
+  Name = ND->getIdentifier();

ricejasonf wrote:

ResolvedUnexpandedPackExpr is not directly associated with a name. My original 
implementation was not for structured bindings so it is a little more general.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++2b %s -verify
+// expected-no-diagnostics
+
+template 
+struct type_ { };
+
+template 
+auto sum(T... t) { return (t + ...); }
+
+struct my_struct {
+   int a;
+   int b;
+   int c;
+  int d;
+};
+
+struct fake_tuple {
+  int arr[4] = {1, 2, 3, 6};
+
+  template 
+  int get() {
+return arr[i];
+  }
+};
+
+namespace std {
+  template 
+  struct tuple_size;
+  template 
+  struct tuple_element;
+
+  template <>
+  struct tuple_size {
+static constexpr unsigned value = 4;
+  };
+
+  template 
+  struct tuple_element {
+using type = int;
+  };
+}
+
+
+template 
+void decompose_tuple() {
+  auto tup = T{{1, 2, 3, 6}};
+  auto&& [x, ...rest, y] = tup;
+
+  ((void)type_(type_{}), ...);
+
+  T arrtup[2] = {T{{1, 2, 3, 6}},
+ T{{7, 9, 10, 11}}};
+  int sum = 0;
+  for (auto [...xs] : arrtup) {
+sum += (xs + ...);
+  }
+}
+
+template 
+void decompose_struct() {
+  T obj{1, 2, 3, 6};
+  auto [x, ...rest, y] = obj;
+}
+
+template 
+void decompose_array() {
+  // previously unable to use non-dependent array here
+  // Fixes https://bugs.llvm.org/show_bug.cgi?id=45964

ricejasonf wrote:

It was me that submitted that fix which I broke out from this. I can remove the 
comment if you want.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -237,7 +237,7 @@ bool Decl::isTemplateParameterPack() const {
 }
 
 bool Decl::isParameterPack() const {
-  if (const auto *Var = dyn_cast(this))
+  if (const auto *Var = dyn_cast(this))

ricejasonf wrote:

They would still both just look for a PackExpansionType. There is no place 
where a variable can be expanded that a binding cannot so I think it benefits 
to have this in ValueDecl. See its use in SemaTemplateVariadic.cpp.

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Add stuctured binding packs (PR #121417)

[Jason Rice via cfe-commits]

@@ -104,7 +104,6 @@ void CodeGenFunction::EmitDecl(const Decl &D) {
   case Decl::Binding:
   case Decl::UnresolvedUsingIfExists:
   case Decl::HLSLBuffer:
-llvm_unreachable("Declaration should not be in declstmts!");

ricejasonf wrote:

I will just put this back in. It has to be an accident, or I do not remember 
why I removed this. :sweat_smile: 

https://github.com/llvm/llvm-project/pull/121417
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Fix invalid pack binding crash (PR #135129)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf created 
https://github.com/llvm/llvm-project/pull/135129

Fixes #134882 

Consider
```
struct foo { char a; int b; };
constexpr foo t{'a', 1};
constexpr auto [...m] = t;
```
Without the `constexpr` qualifier, the decomposition declaration just happens 
to not crash in the call to `DeclMustBeEmitted` because it returns early 
because of its "discardable gval linkage". So, the fix is in `flat_bindings` 
where we cannot assume the pack binding is valid. There is also a fix along the 
same vein from a suggestion made by @shafik. The tests are still building on my 
machine, but I thought I would submit this to get eyes on it earlier since it 
is trivial.

>From 27290e3c7dc2a36a33e4582758d875704c7cbe33 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Wed, 9 Apr 2025 21:31:00 -0700
Subject: [PATCH] [Clang][P1061] Fix invalid pack binding crash

---
 clang/include/clang/AST/DeclCXX.h | 2 +-
 clang/lib/Sema/SemaTemplateInstantiateDecl.cpp| 5 ++---
 clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp | 6 ++
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 56cec07ec0293..ba6d87f8158ab 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4281,7 +4281,7 @@ class DecompositionDecl final
 [](BindingDecl *BD) { return BD->isParameterPack(); });
 
 Bindings = Bindings.drop_front(BeforePackBindings.size());
-if (!Bindings.empty()) {
+if (!Bindings.empty() && Bindings.front()->getBinding()) {
   PackBindings = Bindings.front()->getBindingPackDecls();
   Bindings = Bindings.drop_front();
 }
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index e0f7ccc4674d8..4f7410c9bf0d8 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1597,11 +1597,10 @@ Decl 
*TemplateDeclInstantiator::VisitDecompositionDecl(DecompositionDecl *D) {
   auto *NewDD = cast_if_present(
   VisitVarDecl(D, /*InstantiatingVarTemplate=*/false, &NewBindingArray));
 
-  if (!NewDD || NewDD->isInvalidDecl())
+  if (!NewDD || NewDD->isInvalidDecl()) {
 for (auto *NewBD : NewBindings)
   NewBD->setInvalidDecl();
-
-  if (OldBindingPack) {
+  } else if (OldBindingPack) {
 // Mark the bindings in the pack as instantiated.
 auto Bindings = NewDD->bindings();
 BindingDecl *NewBindingPack = *llvm::find_if(
diff --git a/clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp 
b/clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
index ea94757dc66b6..1818dc699c71c 100644
--- a/clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
+++ b/clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
@@ -8,4 +8,10 @@ void decompose_array() {
   // cxx23-warning@+2 {{structured binding packs are a C++2c extension}}
   // nontemplate-error@+1 {{pack declaration outside of template}}
   auto [x, ...rest, y] = arr;
+
+  // cxx26-warning@+4 {{structured binding packs are incompatible with C++ 
standards before C++2c}}
+  // cxx23-warning@+3 {{structured binding packs are a C++2c extension}}
+  // nontemplate-error@+2 {{decomposition declaration cannot be declared 
'constexpr'}}
+  // nontemplate-error@+1 {{pack declaration outside of template}}
+  constexpr auto [x, ...rest, y] = arr;
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Fix invalid pack binding crash (PR #135129)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/135129

>From 1597be864b8617a6e325590b742746572df0d78e Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Wed, 9 Apr 2025 21:31:00 -0700
Subject: [PATCH] [Clang][P1061] Fix invalid pack binding crash

---
 clang/include/clang/AST/DeclCXX.h | 2 +-
 clang/lib/Sema/SemaTemplateInstantiateDecl.cpp| 5 ++---
 clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp | 8 +++-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/clang/include/clang/AST/DeclCXX.h 
b/clang/include/clang/AST/DeclCXX.h
index 56cec07ec0293..ba6d87f8158ab 100644
--- a/clang/include/clang/AST/DeclCXX.h
+++ b/clang/include/clang/AST/DeclCXX.h
@@ -4281,7 +4281,7 @@ class DecompositionDecl final
 [](BindingDecl *BD) { return BD->isParameterPack(); });
 
 Bindings = Bindings.drop_front(BeforePackBindings.size());
-if (!Bindings.empty()) {
+if (!Bindings.empty() && Bindings.front()->getBinding()) {
   PackBindings = Bindings.front()->getBindingPackDecls();
   Bindings = Bindings.drop_front();
 }
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index e0f7ccc4674d8..4f7410c9bf0d8 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1597,11 +1597,10 @@ Decl 
*TemplateDeclInstantiator::VisitDecompositionDecl(DecompositionDecl *D) {
   auto *NewDD = cast_if_present(
   VisitVarDecl(D, /*InstantiatingVarTemplate=*/false, &NewBindingArray));
 
-  if (!NewDD || NewDD->isInvalidDecl())
+  if (!NewDD || NewDD->isInvalidDecl()) {
 for (auto *NewBD : NewBindings)
   NewBD->setInvalidDecl();
-
-  if (OldBindingPack) {
+  } else if (OldBindingPack) {
 // Mark the bindings in the pack as instantiated.
 auto Bindings = NewDD->bindings();
 BindingDecl *NewBindingPack = *llvm::find_if(
diff --git a/clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp 
b/clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
index ea94757dc66b6..a4f0bcdb4270b 100644
--- a/clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
+++ b/clang/test/SemaCXX/cxx2c-binding-pack-nontemplate.cpp
@@ -3,9 +3,15 @@
 // RUN: %clang_cc1 -std=c++23 -verify=cxx23,nontemplate -fsyntax-only 
-Wc++26-extensions %s
 
 void decompose_array() {
-  int arr[4] = {1, 2, 3, 6};
+  constexpr int arr[4] = {1, 2, 3, 6};
   // cxx26-warning@+3 {{structured binding packs are incompatible with C++ 
standards before C++2c}}
   // cxx23-warning@+2 {{structured binding packs are a C++2c extension}}
   // nontemplate-error@+1 {{pack declaration outside of template}}
   auto [x, ...rest, y] = arr;
+
+  // cxx26-warning@+4 {{structured binding packs are incompatible with C++ 
standards before C++2c}}
+  // cxx23-warning@+3 {{structured binding packs are a C++2c extension}}
+  // nontemplate-error@+2 {{decomposition declaration cannot be declared 
'constexpr'}}
+  // nontemplate-error@+1 {{pack declaration outside of template}}
+  constexpr auto [x_c, ...rest_c, y_c] = arr;
 }

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Fix invalid pack binding crash (PR #135129)

[Jason Rice via cfe-commits]

ricejasonf wrote:

Yes, please, merge it when you can.

https://github.com/llvm/llvm-project/pull/135129
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Fix invalid pack binding crash (PR #135129)

[Jason Rice via cfe-commits]

ricejasonf wrote:

Sorry, I haven't had time to revisit this, but in that test we are only 
checking the validity of the decomposition declarations so I think the addition 
of `constexpr` to that variable should not affect the checks on the other 
declarations. There is plenty of coverage on decompositions in the other not 
"nontemplate" test file. Still, I probably did not need to step on the original 
test like that. I will consider adding more "invalid declaration" tests and 
address this when I get more time. Thank you.

https://github.com/llvm/llvm-project/pull/135129
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Fix template arguments in local classes (PR #121225)

[Jason Rice via cfe-commits]

@@ -4433,8 +4433,12 @@ Sema::InstantiateClassMembers(SourceLocation 
PointOfInstantiation,
   // No need to instantiate in-class initializers during explicit
   // instantiation.
   if (Field->hasInClassInitializer() && TSK == TSK_ImplicitInstantiation) {
+// Handle local classes which could have substituted template params.
 CXXRecordDecl *ClassPattern =
-Instantiation->getTemplateInstantiationPattern();
+Instantiation->isLocalClass()
+? Instantiation->getInstantiatedFromMemberClass()
+: Instantiation->getTemplateInstantiationPattern();

ricejasonf wrote:

The comments for that states 
```cpp
  /// Retrieve the record declaration from which this record could be
  /// instantiated. Returns null if this class is not a template instantiation.
  const CXXRecordDecl *getTemplateInstantiationPattern() const;
```

So the "pattern" as I read it, is not an instantiation itself but the template 
(or partial specialization). In the case of the field in  `struct L` we need 
the intermediate instantiated struct to get the instantiated field.

https://github.com/llvm/llvm-project/pull/121225
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Fix template arguments in local classes (PR #121225)

[Jason Rice via cfe-commits]

https://github.com/ricejasonf updated 
https://github.com/llvm/llvm-project/pull/121225

>From 5be8b4b38cb4d595d5e1ec53bdcda3bb24333644 Mon Sep 17 00:00:00 2001
From: Jason Rice 
Date: Mon, 5 Aug 2024 13:53:33 -0700
Subject: [PATCH] [Clang][P1061] Fix template arguments in local classes

---
 clang/docs/ReleaseNotes.rst|  1 +
 clang/lib/Sema/SemaTemplateInstantiate.cpp |  6 +-
 .../SemaCXX/local-class-template-param-crash.cpp   | 14 ++
 3 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/local-class-template-param-crash.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8f72553acfa4c..8ba493b2ca89b 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -955,6 +955,7 @@ Bug Fixes to C++ Support
   consistently treat the initializer as manifestly constant-evaluated.
   (#GH135281)
 - Fix a crash in the presence of invalid base classes. (#GH147186)
+- Fix a crash with NTTP when instantiating local class.
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index f04b01f64b960..20bac0e56b195 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -4412,8 +4412,12 @@ Sema::InstantiateClassMembers(SourceLocation 
PointOfInstantiation,
   // No need to instantiate in-class initializers during explicit
   // instantiation.
   if (Field->hasInClassInitializer() && TSK == TSK_ImplicitInstantiation) {
+// Handle local classes which could have substituted template params.
 CXXRecordDecl *ClassPattern =
-Instantiation->getTemplateInstantiationPattern();
+Instantiation->isLocalClass()
+? Instantiation->getInstantiatedFromMemberClass()
+: Instantiation->getTemplateInstantiationPattern();
+
 DeclContext::lookup_result Lookup =
 ClassPattern->lookup(Field->getDeclName());
 FieldDecl *Pattern = Lookup.find_first();
diff --git a/clang/test/SemaCXX/local-class-template-param-crash.cpp 
b/clang/test/SemaCXX/local-class-template-param-crash.cpp
new file mode 100644
index 0..ffa8590eaf77d
--- /dev/null
+++ b/clang/test/SemaCXX/local-class-template-param-crash.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// expected-no-diagnostics
+
+template 
+int g() {
+  return [] (auto) -> int {
+struct L {
+  int m = i;
+};
+return 0;
+  } (42);
+}
+
+int v = g<1>();

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][P1061] Fix template arguments in local classes (PR #121225)

[Jason Rice via cfe-commits]

ricejasonf wrote:

I verified that it is still broken on trunk (on godbolt). It is now rebased on 
`main` with a shiny new release note. Please merge it if you think it is the 
right fix (and the tests finish). @cor3ntin 

https://github.com/llvm/llvm-project/pull/121225
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits