[Clang][P1061] Remove BindingInitWalker

Author: ricejasonf

clang/include/clang/AST/Expr.h

@@ -1482,6 +1482,12 @@ class DeclRefExpr final
     setDependence(computeDependence(this, Context));
   }
 
+  /// Set type and compute and set dependence bits.
+  void setTypeFromBinding(QualType T, const ASTContext &Context) {
+    setType(T.getNonReferenceType());
+    setDependence(computeDependence(this, Context));
+  }
+
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == DeclRefExprClass;
   }

clang/lib/Sema/SemaDeclCXX.cpp

@@ -959,10 +959,8 @@ 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
+// Check the arity of the structured bindings.
+// Create the resolved pack expr if needed.
 static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
                                QualType DecompType,
                                ArrayRef<BindingDecl *> Bindings,
@@ -975,19 +973,31 @@ static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
   if (!HasPack) {
     IsValid = Bindings.size() == MemberCount;
   } else {
-    // there may not be more members than non-pack bindings
+    // 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
+    // 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));
+    auto *RP = ResolvedUnexpandedPackExpr::Create(S.Context, DD->getBeginLoc(),
+                                                  DecompType, PackSize);
+    BD->setDecomposedDecl(DD);
+    BD->setBinding(PackType, RP);
+
+    BindingDecl *BPack = *BindingWithPackItr;
+    // Create the nested BindingDecls.
+    for (Expr *&E : RP->getExprs()) {
+      auto *NestedBD = BindingDecl::Create(S.Context, BPack->getDeclContext(),
+                                           BPack->getLocation(),
+                                           BPack->getIdentifier(), QualType());
+      NestedBD->setDecomposedDecl(DD);
+      E = S.BuildDeclRefExpr(NestedBD, S.Context.DependentTy, VK_LValue,
+                             BPack->getLocation());
+    }
   }
 
   if (IsValid)
@@ -999,84 +1009,26 @@ static bool CheckBindingsCount(Sema &S, DecompositionDecl *DD,
   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
-struct BindingInitWalker {
-  using BindingItrTy = typename ArrayRef<BindingDecl *>::iterator;
-  using PackExprItrTy = typename MutableArrayRef<Expr *>::iterator;
-  Sema &SemaRef;
-  DecompositionDecl *DecompDecl;
-  ArrayRef<BindingDecl *> Bindings;
-  ResolvedUnexpandedPackExpr *PackExpr = nullptr;
-  MutableArrayRef<Expr *> PackExprNodes;
-  BindingItrTy BindingItr;
-  PackExprItrTy PackExprItr;
-
-  BindingInitWalker(Sema &S, DecompositionDecl *DD, ArrayRef<BindingDecl *> Bs)
-      : SemaRef(S), DecompDecl(DD), Bindings(Bs), BindingItr(Bindings.begin()) {
-  }
-
-  BindingDecl *get() { return *BindingItr; }
-
-  void commitAndAdvance(QualType T, Expr *E) {
-    BindingDecl *B = *BindingItr;
-    bool IsPackExpr =
-        isa_and_nonnull<ResolvedUnexpandedPackExpr>(B->getBinding());
-    if (IsPackExpr && !PackExpr) {
-      PackExpr = cast<ResolvedUnexpandedPackExpr>(B->getBinding());
-      PackExprNodes = PackExpr->getExprs();
-      PackExprItr = PackExprNodes.begin();
-    }
-
-    if (IsPackExpr) {
-      // Build a nested BindingDecl with a DeclRefExpr
-      auto *NestedBD =
-          BindingDecl::Create(SemaRef.Context, B->getDeclContext(),
-                              B->getLocation(), B->getIdentifier(), T);
-
-      NestedBD->setBinding(T, E);
-      NestedBD->setDecomposedDecl(DecompDecl);
-      auto *DE = SemaRef.BuildDeclRefExpr(NestedBD, T.getNonReferenceType(),
-                                          VK_LValue, B->getLocation());
-      *PackExprItr = DE;
-      if (++PackExprItr != PackExprNodes.end())
-        return;
-      // If we hit the end of the pack exprs then
-      // continue to advance BindingItr
-    } else {
-      (*BindingItr)->setBinding(T, E);
-    }
-
-    ++BindingItr;
-  }
-};
-
 static bool checkSimpleDecomposition(
     Sema &S, ArrayRef<BindingDecl *> Bindings, ValueDecl *Src,
     QualType DecompType, const llvm::APSInt &NumElemsAPS, QualType ElemType,
     llvm::function_ref<ExprResult(SourceLocation, Expr *, unsigned)> GetInit) {
   unsigned NumElems = (unsigned)NumElemsAPS.getLimitedValue(UINT_MAX);
+  auto *DD = cast<DecompositionDecl>(Src);
 
-  if (CheckBindingsCount(S, cast<DecompositionDecl>(Src), DecompType, Bindings,
-                         NumElems)) {
-
+  if (CheckBindingsCount(S, DD, DecompType, Bindings, NumElems))
     return true;
-  }
 
-  auto Walker = BindingInitWalker(S, cast<DecompositionDecl>(Src), Bindings);
-  for (unsigned I = 0; I < NumElems; I++) {
-    BindingDecl *B = Walker.get();
+  unsigned I = 0;
+  for (auto *B : DD->flat_bindings()) {
     SourceLocation Loc = B->getLocation();
     ExprResult E = S.BuildDeclRefExpr(Src, DecompType, VK_LValue, Loc);
     if (E.isInvalid())
       return true;
-    E = GetInit(Loc, E.get(), I);
+    E = GetInit(Loc, E.get(), I++);
     if (E.isInvalid())
       return true;
-    Walker.commitAndAdvance(ElemType, E.get());
+    B->setBinding(ElemType, E.get());
   }
 
   return false;
@@ -1314,11 +1266,10 @@ static bool checkTupleLikeDecomposition(Sema &S,
                                         ArrayRef<BindingDecl *> Bindings,
                                         VarDecl *Src, QualType DecompType,
                                         const llvm::APSInt &TupleSize) {
+  auto *DD = cast<DecompositionDecl>(Src);
   unsigned NumElems = (unsigned)TupleSize.getLimitedValue(UINT_MAX);
-  if (CheckBindingsCount(S, cast<DecompositionDecl>(Src), DecompType, Bindings,
-                         NumElems)) {
+  if (CheckBindingsCount(S, DD, DecompType, Bindings, NumElems))
     return true;
-  }
 
   if (Bindings.empty())
     return false;
@@ -1351,9 +1302,8 @@ static bool checkTupleLikeDecomposition(Sema &S,
     }
   }
 
-  auto Walker = BindingInitWalker(S, cast<DecompositionDecl>(Src), Bindings);
-  for (unsigned I = 0; I < NumElems; I++) {
-    BindingDecl *B = Walker.get();
+  unsigned I = 0;
+  for (auto *B : DD->flat_bindings()) {
     InitializingBinding InitContext(S, B);
     SourceLocation Loc = B->getLocation();
 
@@ -1439,7 +1389,8 @@ static bool checkTupleLikeDecomposition(Sema &S,
     if (E.isInvalid())
       return true;
 
-    Walker.commitAndAdvance(T, E.get());
+    B->setBinding(T, E.get());
+    I++;
   }
 
   return false;
@@ -1535,18 +1486,18 @@ static bool checkMemberDecomposition(Sema &S, ArrayRef<BindingDecl*> Bindings,
   QualType BaseType = S.Context.getQualifiedType(S.Context.getRecordType(RD),
                                                  DecompType.getQualifiers());
 
+  auto *DD = cast<DecompositionDecl>(Src);
   unsigned NumFields = llvm::count_if(
       RD->fields(), [](FieldDecl *FD) { return !FD->isUnnamedBitField(); });
-  if (CheckBindingsCount(S, cast<DecompositionDecl>(Src), DecompType, Bindings,
-                         NumFields)) {
+  if (CheckBindingsCount(S, DD, DecompType, Bindings, NumFields))
     return true;
-  }
-
-  auto Walker = BindingInitWalker(S, cast<DecompositionDecl>(Src), Bindings);
 
   //   all of E's non-static data members shall be [...] well-formed
   //   when named as e.name in the context of the structured binding,
   //   E shall not have an anonymous union member, ...
+  auto FlatBindings = DD->flat_bindings();
+  assert(llvm::range_size(FlatBindings) == NumFields);
+  auto FlatBindingsItr = FlatBindings.begin();
   for (auto *FD : RD->fields()) {
     if (FD->isUnnamedBitField())
       continue;
@@ -1571,7 +1522,8 @@ static bool checkMemberDecomposition(Sema &S, ArrayRef<BindingDecl*> Bindings,
     }
 
     // We have a real field to bind.
-    BindingDecl *B = Walker.get();
+    assert(FlatBindingsItr != FlatBindings.end());
+    BindingDecl *B = *(FlatBindingsItr++);
     SourceLocation Loc = B->getLocation();
 
     // The field must be accessible in the context of the structured binding.
@@ -1606,8 +1558,7 @@ static bool checkMemberDecomposition(Sema &S, ArrayRef<BindingDecl*> Bindings,
     Qualifiers Q = DecompType.getQualifiers();
     if (FD->isMutable())
       Q.removeConst();
-    Walker.commitAndAdvance(S.BuildQualifiedType(FD->getType(), Loc, Q),
-                            E.get());
+    B->setBinding(S.BuildQualifiedType(FD->getType(), Loc, Q), E.get());
   }
 
   return false;
@@ -1625,6 +1576,19 @@ void Sema::CheckCompleteDecompositionDeclaration(DecompositionDecl *DD) {
         B->setType(Context.DependentTy);
     }
     return;
+  } else {
+    // Set the types of the DeclRefExprs that point to nested pack bindings.
+    for (BindingDecl *B : DD->bindings()) {
+      if (B->isParameterPack()) {
+        for (Expr *E : B->getBindingPackExprs()) {
+          auto *DRE = cast<DeclRefExpr>(E);
+          auto *NestedB = cast<BindingDecl>(DRE->getDecl());
+          QualType T = NestedB->getType();
+          DRE->setTypeFromBinding(T, Context);
+        }
+        break;
+      }
+    }
   }
 
   DecompType = DecompType.getNonReferenceType();

clang/test/Parser/cxx2c-binding-pack.cpp

@@ -6,6 +6,20 @@ void decompose_array() {
   auto [x, ... // #1
     rest, ...more_rest] = arr; // expected-error{{multiple packs in structured binding declaration}}
                                // expected-note@#1{{previous binding pack specified here}}
-                               //
+
   auto [y...] = arr; // expected-error{{'...' must immediately precede declared identifier}}
+
+  auto [...] = arr; // #2
+                    // expected-error@#2{{expected identifier}}
+                    // expected-error@#2{{{no names were provided}}}
+                    // expected-warning@#2{{{does not allow a decomposition group to be empty}}}
+  auto [a, ..., b] = arr; // #3
+                          // expected-error@#3{{expected identifier}}
+                          // expected-error@#3{{{only 1 name was provided}}}
+  auto [a1, ...] = arr; // #4
+                        // expected-error@#4{{expected identifier}}
+                        // expected-error@#4{{{only 1 name was provided}}}
+  auto [..., b] = arr; // #5
+                       // expected-error@#5{{expected identifier}}
+                       // expected-error@#5{{{no names were provided}}}
 }

clang/test/SemaCXX/cxx2c-binding-pack.cpp

@@ -1,5 +1,4 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++26 %s -verify
-// expected-no-diagnostics
 
 template <typename T>
 struct type_ { };
@@ -8,9 +7,9 @@ template <typename ...T>
 auto sum(T... t) { return (t + ...); }
 
 struct my_struct {
-	int a;
-	int b;
-	int c;
+  int a;
+  int b;
+  int c;
   int d;
 };
 
@@ -74,6 +73,14 @@ void decompose_array() {
   int size = sizeof...(rest);
   T arr2[sizeof...(rest)] = {rest...};
   auto [...pack] = arr2;
+
+  // Array of size 1.
+  int arr1[1] = {1};
+  auto [a, ...b] = arr1;
+  static_assert(sizeof...(b) == 0);
+  auto [...c] = arr1;
+  static_assert(sizeof...(c) == 1);
+  auto [a1, ...b1, c1] = arr1; // expected-error{{decomposes into 1 element, but 3 names were provided}}
 }
 
 // Test case by Younan Zhang.
@@ -115,3 +122,57 @@ int main() {
   S<1, 2, 3, 4>::N<5, 6>().foo();
   decompose_bit_field<bit_fields>();
 }
+
+// P1061R10 Stuff
+namespace {
+struct C { int x, y, z; };
+
+template <class T>
+void now_i_know_my() {
+  auto [a, b, c] = C(); // OK, SB0 is a, SB1 is b, and SB2 is c
+  auto [d, ...e] = C(); // OK, SB0 is d, the pack e (v1) contains two structured bindings: SB1 and SB2
+  static_assert(sizeof...(e) == 2);
+  auto [...f, g] = C(); // OK, the pack f (v0) contains two structured bindings: SB0 and SB1, and SB2 is g
+  static_assert(sizeof...(e) == 2);
+  auto [h, i, j, ...k] = C(); // OK, the pack k is empty
+  static_assert(sizeof...(e) == 0);
+  auto [l, m, n, o, ...p] = C(); // expected-error{{{decomposes into 3 elements, but 5 names were provided}}}
+}
+}  // namespace
+
+namespace {
+auto g() -> int(&)[4];
+
+template <unsigned long N>
+void h(int (&arr)[N]) {
+  auto [a, ...b, c] = arr;  // a names the first element of the array,
+                            // b is a pack referring to the second and
+                            // third elements, and c names the fourth element
+  static_assert(sizeof...(b) == 2);
+ auto& [...e] = arr;        // e is a pack referring to the four elements of the array
+  static_assert(sizeof...(e) == 4);
+}
+
+void call_h() {
+ h(g());
+}
+}  // namespace
+
+namespace {
+struct D { };
+
+int g(...) { return 1; }
+
+template <typename T>
+constexpr int f() {
+  D arr[1];
+  auto [...e] = arr;
+  return g(e...);
+}
+
+constexpr int g(D) { return 2; }
+
+void other_main() {
+  static_assert(f<int>() == 2);
+}
+}  // namespace