[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao updated this revision to Diff 550927.
SlaterLatiao added a comment.

- Refactor instantiating members into 2 functions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenCXX/data_member_packs.cpp

Index: clang/test/CodeGenCXX/data_member_packs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/data_member_packs.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 --std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s --check-prefixes=CHECK
+
+// Tests declaration data member packs.
+template struct S1 {
+Ts... ts;
+};
+
+template struct S2 {
+T t[2];
+Ts... ts;
+};
+
+// CHECK: %struct.S1 = type { i32 }
+S1 s1;
+// CHECK-NEXT: %struct.S1.0 = type { i32, float, double }
+S1 s2;
+// Test template args as the last arg.
+// CHECK-NEXT: %struct.S2 = type { [2 x i32], float, double }
+S2 s3;
+// Test nested template args.
+// CHECK-NEXT: %struct.S1.1 = type { i32, float, %struct.S1.2 }
+// CHECK-NEXT: %struct.S1.2 = type { double, double }
+S1> s4;
+// Test empty template arg.
+// CHECK-NEXT: %struct.S1.3 = type { i8 }
+S1<> s5;
+// Test duplicate types in template args.
+// CHECK-NEXT: %struct.S1.4 = type { i32, i32 }
+S1 s6;
+
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5927,6 +5927,9 @@
  /*ExpectPackInType=*/false);
   }
   break;
+case DeclaratorContext::Member:
+  // Expand for data member packs.
+  // https://discourse.llvm.org/t/adding-support-for-data-member-packs/71333
 case DeclaratorContext::TemplateParam:
   // C++0x [temp.param]p15:
   //   If a template-parameter is a [...] is a parameter-declaration that
@@ -5954,7 +5957,6 @@
 case DeclaratorContext::CXXNew:
 case DeclaratorContext::AliasDecl:
 case DeclaratorContext::AliasTemplate:
-case DeclaratorContext::Member:
 case DeclaratorContext::Block:
 case DeclaratorContext::ForInit:
 case DeclaratorContext::SelectionInit:
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3135,6 +3135,93 @@
   }
 }
 
+/// Instantiate the member pack of a class.
+void InstantiateMemberPack(Sema &S, CXXRecordDecl *Instantiation,
+   const MultiLevelTemplateArgumentList &TemplateArgs,
+   FieldDecl *Field, SmallVector &Fields,
+   TemplateDeclInstantiator &Instantiator) {
+  QualType PatternType =
+  Field->getType()->castAs()->getPattern();
+  std::optional NumArgumentsInExpansion =
+  S.getNumArgumentsInExpansion(Field->getType(), TemplateArgs);
+  assert(NumArgumentsInExpansion &&
+ "should not see unknown template argument here");
+  for (unsigned Arg = 0; Arg < *NumArgumentsInExpansion; ++Arg) {
+// Generate a new field from PackExpansion field.
+if (Decl *NewMember = Instantiator.Visit(Field)) {
+  FieldDecl *PackedField = cast(NewMember);
+  Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(S, Arg);
+  QualType T =
+  S.SubstType(PatternType, TemplateArgs, PackedField->getLocation(),
+  PackedField->getDeclName());
+  PackedField->setType(T);
+  Fields.push_back(PackedField);
+  if (NewMember->isInvalidDecl()) {
+// When `NewMember` has type of `PackExpansionType`, it escapes
+// validation checks in `Visit`. Handling of such cases
+// will be implemented in a future commit.
+// Currently this branch should never be reached.
+assert(false && "not implemented");
+Instantiation->setInvalidDecl();
+  }
+} else {
+  // FIXME: This is the same situation of InstantiateMember, when handling
+  // non-pack members.
+  continue;
+}
+  }
+}
+
+/// Instantiate the non-pack members of a class.
+///
+/// \returns true if need to bail out the member instantiation ,loop, false otherwise.
+bool InstantiateMember(Sema &S, SourceLocation &PointOfInstantiation,
+   CXXRecordDecl *Instantiation, Decl *Member,
+   TemplateSpecializationKind TSK,
+   SmallVector &Fields,
+   TemplateDeclInstantiator &Instantiator,
+   bool &MightHaveConstexprVirtualFunctions) {
+  Decl *NewMember = Instantiator.Visit(Member);
+  if (NewMember) {
+if (FieldDecl *Field = dyn_cast(NewMember)) {
+  Fields.push_back(Field);
+} else if (EnumDecl *Enum = dyn_cast(NewMember)) {
+  // C++11 [temp.inst]p1: The implicit i

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao updated this revision to Diff 545286.
SlaterLatiao added a comment.

- Address review comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

Files:
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/ast-print-method-decl.cpp
  clang/test/CodeGenCXX/data_member_packs.cpp

Index: clang/test/CodeGenCXX/data_member_packs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/data_member_packs.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 --std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s --check-prefixes=CHECK
+
+// Tests declaration data member packs.
+template struct S1 {
+Ts... ts;
+};
+
+template struct S2 {
+T t[2];
+Ts... ts;
+};
+
+// CHECK: %struct.S1 = type { i32 }
+S1 s1;
+// CHECK-NEXT: %struct.S1.0 = type { i32, float, double }
+S1 s2;
+// Test template args as the last arg.
+// CHECK-NEXT: %struct.S2 = type { [2 x i32], float, double }
+S2 s3;
+// Test nested template args.
+// CHECK-NEXT: %struct.S1.1 = type { i32, float, %struct.S1.2 }
+// CHECK-NEXT: %struct.S1.2 = type { double, double }
+S1> s4;
+// Test empty template arg.
+// CHECK-NEXT: %struct.S1.3 = type { i8 }
+S1<> s5;
+// Test duplicate types in template args.
+// CHECK-NEXT: %struct.S1.4 = type { i32, i32 }
+S1 s6;
Index: clang/test/AST/ast-print-method-decl.cpp
===
--- clang/test/AST/ast-print-method-decl.cpp
+++ clang/test/AST/ast-print-method-decl.cpp
@@ -85,3 +85,18 @@
 
   // CHECK-NEXT: };
 };
+
+
+// CHECK: struct DefMethodsWithoutBody {
+struct DefMethodsWithoutBody {
+  // CHECK-NEXT: DefMethodsWithoutBody() = delete;
+  DefMethodsWithoutBody() = delete;
+
+  // CHECK-NEXT: DefMethodsWithoutBody() = default;
+  ~DefMethodsWithoutBody() = default;
+
+  // CHECK-NEXT: void m1() __attribute__((alias("X")));
+  void m1() __attribute__((alias("X")));
+
+  // CHECK-NEXT: };
+};
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5927,6 +5927,8 @@
  /*ExpectPackInType=*/false);
   }
   break;
+case DeclaratorContext::Member:
+  // Expand for packed data members.
 case DeclaratorContext::TemplateParam:
   // C++0x [temp.param]p15:
   //   If a template-parameter is a [...] is a parameter-declaration that
@@ -5944,7 +5946,7 @@
  ? diag::warn_cxx98_compat_variadic_templates
  : diag::ext_variadic_templates);
   break;
-
+  
 case DeclaratorContext::File:
 case DeclaratorContext::KNRTypeList:
 case DeclaratorContext::ObjCParameter: // FIXME: special diagnostic here?
@@ -5954,7 +5956,6 @@
 case DeclaratorContext::CXXNew:
 case DeclaratorContext::AliasDecl:
 case DeclaratorContext::AliasTemplate:
-case DeclaratorContext::Member:
 case DeclaratorContext::Block:
 case DeclaratorContext::ForInit:
 case DeclaratorContext::SelectionInit:
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3267,42 +3267,74 @@
   continue;
 }
 
-Decl *NewMember = Instantiator.Visit(Member);
-if (NewMember) {
-  if (FieldDecl *Field = dyn_cast(NewMember)) {
-Fields.push_back(Field);
-  } else if (EnumDecl *Enum = dyn_cast(NewMember)) {
-// C++11 [temp.inst]p1: The implicit instantiation of a class template
-// specialization causes the implicit instantiation of the definitions
-// of unscoped member enumerations.
-// Record a point of instantiation for this implicit instantiation.
-if (TSK == TSK_ImplicitInstantiation && !Enum->isScoped() &&
-Enum->isCompleteDefinition()) {
-  MemberSpecializationInfo *MSInfo =Enum->getMemberSpecializationInfo();
-  assert(MSInfo && "no spec info for member enum specialization");
-  MSInfo->setTemplateSpecializationKind(TSK_ImplicitInstantiation);
-  MSInfo->setPointOfInstantiation(PointOfInstantiation);
-}
-  } else if (StaticAssertDecl *SA = dyn_cast(NewMember)) {
-if (SA->isFailed()) {
-  // A static_assert failed. Bail out; instantiating this
-  // class is probably not meaningful.
-  Instantiation->setInvalidDecl();
-  break;
+// Instantiate packed data members.
+if (FieldDecl *Field = dyn_cast(Member);
+Field && isa(Field->getType().getTypePtr())) {
+  QualType PatternType = Field->getType()
+ ->castAs()
+ ->getPattern();
+  std

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao updated this revision to Diff 545291.
SlaterLatiao marked 2 inline comments as done.
SlaterLatiao added a comment.

Revert last commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenCXX/packed_data_member.cpp

Index: clang/test/CodeGenCXX/packed_data_member.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/packed_data_member.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 --std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s --check-prefixes=CHECK
+
+// Tests declaration of packed data members.
+template struct S1 {
+Ts... ts;
+};
+
+template struct S2 {
+T t[2];
+Ts... ts;
+};
+
+// CHECK: %struct.S1 = type { i32 }
+S1 s1;
+// CHECK-NEXT: %struct.S1.0 = type { i32, float, double }
+S1 s2;
+// Test template args as the last arg.
+// CHECK-NEXT: %struct.S2 = type { [2 x i32], float, double }
+S2 s3;
+// Test nested template args.
+// CHECK-NEXT: %struct.S1.1 = type { i32, float, %struct.S1.2 }
+// CHECK-NEXT: %struct.S1.2 = type { double, double }
+S1> s4;
+// Test empty template arg.
+// CHECK-NEXT: %struct.S1.3 = type { i8 }
+S1<> s5;
\ No newline at end of file
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5927,6 +5927,8 @@
  /*ExpectPackInType=*/false);
   }
   break;
+case DeclaratorContext::Member:
+  // Expand for packed data members.
 case DeclaratorContext::TemplateParam:
   // C++0x [temp.param]p15:
   //   If a template-parameter is a [...] is a parameter-declaration that
@@ -5944,7 +5946,6 @@
  ? diag::warn_cxx98_compat_variadic_templates
  : diag::ext_variadic_templates);
   break;
-
 case DeclaratorContext::File:
 case DeclaratorContext::KNRTypeList:
 case DeclaratorContext::ObjCParameter: // FIXME: special diagnostic here?
@@ -5954,7 +5955,6 @@
 case DeclaratorContext::CXXNew:
 case DeclaratorContext::AliasDecl:
 case DeclaratorContext::AliasTemplate:
-case DeclaratorContext::Member:
 case DeclaratorContext::Block:
 case DeclaratorContext::ForInit:
 case DeclaratorContext::SelectionInit:
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3267,42 +3267,75 @@
   continue;
 }
 
-Decl *NewMember = Instantiator.Visit(Member);
-if (NewMember) {
-  if (FieldDecl *Field = dyn_cast(NewMember)) {
-Fields.push_back(Field);
-  } else if (EnumDecl *Enum = dyn_cast(NewMember)) {
-// C++11 [temp.inst]p1: The implicit instantiation of a class template
-// specialization causes the implicit instantiation of the definitions
-// of unscoped member enumerations.
-// Record a point of instantiation for this implicit instantiation.
-if (TSK == TSK_ImplicitInstantiation && !Enum->isScoped() &&
-Enum->isCompleteDefinition()) {
-  MemberSpecializationInfo *MSInfo =Enum->getMemberSpecializationInfo();
-  assert(MSInfo && "no spec info for member enum specialization");
-  MSInfo->setTemplateSpecializationKind(TSK_ImplicitInstantiation);
-  MSInfo->setPointOfInstantiation(PointOfInstantiation);
-}
-  } else if (StaticAssertDecl *SA = dyn_cast(NewMember)) {
-if (SA->isFailed()) {
-  // A static_assert failed. Bail out; instantiating this
-  // class is probably not meaningful.
-  Instantiation->setInvalidDecl();
-  break;
+// Instantiate packed data members.
+if (FieldDecl *Field = dyn_cast(Member);
+Field && isa(Field->getType().getTypePtr())) {
+  QualType PatternType = Field->getType()
+ ->castAs()
+ ->getPattern();
+  std::optional NumArgumentsInExpansion =
+  getNumArgumentsInExpansion(Field->getType(), TemplateArgs);
+  assert(NumArgumentsInExpansion && "should not see unknown template argument here");
+  for (unsigned Arg = 0; Arg < NumArgumentsInExpansion; ++Arg) {
+// Generate a new field from PackExpansion field.
+Decl *NewMember = Instantiator.Visit(Member);
+if (NewMember) {
+  FieldDecl *PackedField = dyn_cast(NewMember);
+  Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(*this, Arg);
+  QualType T =
+  SubstType(PatternType, TemplateArgs, PackedField->getLocation(),
+PackedField->getDeclName());
+  PackedField->setType(T);
+  Fi

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao updated this revision to Diff 545297.
SlaterLatiao added a comment.

Address comments.

- Added test case of duplicate types in template arguments.
- Added ending new line to test case file.
- Renamed "packed data members" -> "data member packs"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenCXX/data_member_packs.cpp

Index: clang/test/CodeGenCXX/data_member_packs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/data_member_packs.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 --std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s --check-prefixes=CHECK
+
+// Tests declaration data member packs.
+template struct S1 {
+Ts... ts;
+};
+
+template struct S2 {
+T t[2];
+Ts... ts;
+};
+
+// CHECK: %struct.S1 = type { i32 }
+S1 s1;
+// CHECK-NEXT: %struct.S1.0 = type { i32, float, double }
+S1 s2;
+// Test template args as the last arg.
+// CHECK-NEXT: %struct.S2 = type { [2 x i32], float, double }
+S2 s3;
+// Test nested template args.
+// CHECK-NEXT: %struct.S1.1 = type { i32, float, %struct.S1.2 }
+// CHECK-NEXT: %struct.S1.2 = type { double, double }
+S1> s4;
+// Test empty template arg.
+// CHECK-NEXT: %struct.S1.3 = type { i8 }
+S1<> s5;
+// Test duplicate types in template args.
+// CHECK-NEXT: %struct.S1.4 = type { i32, i32 }
+S1 s6;
+
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5927,6 +5927,8 @@
  /*ExpectPackInType=*/false);
   }
   break;
+case DeclaratorContext::Member:
+  // Expand for packed data members.
 case DeclaratorContext::TemplateParam:
   // C++0x [temp.param]p15:
   //   If a template-parameter is a [...] is a parameter-declaration that
@@ -5954,7 +5956,6 @@
 case DeclaratorContext::CXXNew:
 case DeclaratorContext::AliasDecl:
 case DeclaratorContext::AliasTemplate:
-case DeclaratorContext::Member:
 case DeclaratorContext::Block:
 case DeclaratorContext::ForInit:
 case DeclaratorContext::SelectionInit:
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3267,42 +3267,74 @@
   continue;
 }
 
-Decl *NewMember = Instantiator.Visit(Member);
-if (NewMember) {
-  if (FieldDecl *Field = dyn_cast(NewMember)) {
-Fields.push_back(Field);
-  } else if (EnumDecl *Enum = dyn_cast(NewMember)) {
-// C++11 [temp.inst]p1: The implicit instantiation of a class template
-// specialization causes the implicit instantiation of the definitions
-// of unscoped member enumerations.
-// Record a point of instantiation for this implicit instantiation.
-if (TSK == TSK_ImplicitInstantiation && !Enum->isScoped() &&
-Enum->isCompleteDefinition()) {
-  MemberSpecializationInfo *MSInfo =Enum->getMemberSpecializationInfo();
-  assert(MSInfo && "no spec info for member enum specialization");
-  MSInfo->setTemplateSpecializationKind(TSK_ImplicitInstantiation);
-  MSInfo->setPointOfInstantiation(PointOfInstantiation);
-}
-  } else if (StaticAssertDecl *SA = dyn_cast(NewMember)) {
-if (SA->isFailed()) {
-  // A static_assert failed. Bail out; instantiating this
-  // class is probably not meaningful.
-  Instantiation->setInvalidDecl();
-  break;
+// Instantiate packed data members.
+if (FieldDecl *Field = dyn_cast(Member);
+Field && isa(Field->getType().getTypePtr())) {
+  QualType PatternType = Field->getType()
+ ->castAs()
+ ->getPattern();
+  std::optional NumArgumentsInExpansion =
+  getNumArgumentsInExpansion(Field->getType(), TemplateArgs);
+  assert(NumArgumentsInExpansion && "should not see unknown template argument here");
+  for (unsigned Arg = 0; Arg < NumArgumentsInExpansion; ++Arg) {
+// Generate a new field from PackExpansion field.
+if (Decl *NewMember = Instantiator.Visit(Member)) {
+  FieldDecl *PackedField = cast(NewMember);
+  Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(*this, Arg);
+  QualType T =
+  SubstType(PatternType, TemplateArgs, PackedField->getLocation(),
+PackedField->getDeclName());
+  PackedField->setType(T);
+  Fields.push_back(PackedField);
+  if (NewMember->isInvalidDecl())
+Instantiation->setInvalidDecl();
+} else {
+  // FIXME:

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao added a comment.

In D156546#4543697 , @dblaikie wrote:

> (just a side note about the title of this patch: I got "packed data members" 
> confused and thought this was referring to struct packing 
> `__attribute__((packed))` - so perhaps something more like "data member 
> packs" would be a more clear term here?)

I changed the name to "data member packs" in description and in the code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

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

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao added inline comments.



Comment at: clang/test/CodeGenCXX/packed_data_member.cpp:8-12
+template struct S2 {
+T t[2];
+Ts... ts;
+};
+

dblaikie wrote:
> Did this test case come out of any particular bug discovered during 
> implementation?
No. I just wanted to test the case when the packed parameter is not the only 
parameter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

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

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3279
+  assert(NumArgumentsInExpansion && "should not see unknown template 
argument here");
+  for (unsigned Arg = 0; Arg < NumArgumentsInExpansion; ++Arg) {
+// Generate a new field from PackExpansion field.

Not sure if this is a niche opinion, but the `std::optional` operator overloads 
make me a bit uncomfortabel/bit unclear what's happening, and I'd personally 
put a deref in here to make it clearer that this code is assuming (in addition 
to the assert above) the optional is set/present.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3289-3290
+  Fields.push_back(PackedField);
+  if (NewMember->isInvalidDecl())
+Instantiation->setInvalidDecl();
+} else {

Is this codepath tested?



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3292-3295
+  // FIXME: Eventually, a NULL return will mean that one of the
+  // instantiations was a semantic disaster, and we'll want to mark
+  // the declaration invalid. For now, we expect to skip some members
+  // that we can't yet handle.

Worth having a test case showing that/what's broken here?



Comment at: clang/lib/Sema/SemaType.cpp:5930-5931
   break;
+case DeclaratorContext::Member:
+  // Expand for packed data members.
 case DeclaratorContext::TemplateParam:

Perhaps a few more words here would be good - quoting a WG21 proposal paper 
that this code implements, similar to the standard quotations in nearby code. 
(be good to mention the WG21 proposal paper/whatnot in the patch 
description/commit message too)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

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

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:5930-5931
   break;
+case DeclaratorContext::Member:
+  // Expand for packed data members.
 case DeclaratorContext::TemplateParam:

dblaikie wrote:
> Perhaps a few more words here would be good - quoting a WG21 proposal paper 
> that this code implements, similar to the standard quotations in nearby code. 
> (be good to mention the WG21 proposal paper/whatnot in the patch 
> description/commit message too)
I would suggest a link to the discourse discussion instead of [[ 
http://wg21.link/P1858 | P1858]], since that thread talks about the proposal 
and goes into a bit more detail than the [[ 
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1858r2.html#member-packs
 | member packs ]] section of the proposal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

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

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao updated this revision to Diff 545804.
SlaterLatiao added a comment.

Fix code.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenCXX/data_member_packs.cpp

Index: clang/test/CodeGenCXX/data_member_packs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/data_member_packs.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 --std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s --check-prefixes=CHECK
+
+// Tests declaration data member packs.
+template struct S1 {
+Ts... ts;
+};
+
+template struct S2 {
+T t[2];
+Ts... ts;
+};
+
+// CHECK: %struct.S1 = type { i32 }
+S1 s1;
+// CHECK-NEXT: %struct.S1.0 = type { i32, float, double }
+S1 s2;
+// Test template args as the last arg.
+// CHECK-NEXT: %struct.S2 = type { [2 x i32], float, double }
+S2 s3;
+// Test nested template args.
+// CHECK-NEXT: %struct.S1.1 = type { i32, float, %struct.S1.2 }
+// CHECK-NEXT: %struct.S1.2 = type { double, double }
+S1> s4;
+// Test empty template arg.
+// CHECK-NEXT: %struct.S1.3 = type { i8 }
+S1<> s5;
+// Test duplicate types in template args.
+// CHECK-NEXT: %struct.S1.4 = type { i32, i32 }
+S1 s6;
+
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5927,6 +5927,8 @@
  /*ExpectPackInType=*/false);
   }
   break;
+case DeclaratorContext::Member:
+  // Expand for packed data members.
 case DeclaratorContext::TemplateParam:
   // C++0x [temp.param]p15:
   //   If a template-parameter is a [...] is a parameter-declaration that
@@ -5954,7 +5956,6 @@
 case DeclaratorContext::CXXNew:
 case DeclaratorContext::AliasDecl:
 case DeclaratorContext::AliasTemplate:
-case DeclaratorContext::Member:
 case DeclaratorContext::Block:
 case DeclaratorContext::ForInit:
 case DeclaratorContext::SelectionInit:
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3267,42 +3267,74 @@
   continue;
 }
 
-Decl *NewMember = Instantiator.Visit(Member);
-if (NewMember) {
-  if (FieldDecl *Field = dyn_cast(NewMember)) {
-Fields.push_back(Field);
-  } else if (EnumDecl *Enum = dyn_cast(NewMember)) {
-// C++11 [temp.inst]p1: The implicit instantiation of a class template
-// specialization causes the implicit instantiation of the definitions
-// of unscoped member enumerations.
-// Record a point of instantiation for this implicit instantiation.
-if (TSK == TSK_ImplicitInstantiation && !Enum->isScoped() &&
-Enum->isCompleteDefinition()) {
-  MemberSpecializationInfo *MSInfo =Enum->getMemberSpecializationInfo();
-  assert(MSInfo && "no spec info for member enum specialization");
-  MSInfo->setTemplateSpecializationKind(TSK_ImplicitInstantiation);
-  MSInfo->setPointOfInstantiation(PointOfInstantiation);
-}
-  } else if (StaticAssertDecl *SA = dyn_cast(NewMember)) {
-if (SA->isFailed()) {
-  // A static_assert failed. Bail out; instantiating this
-  // class is probably not meaningful.
-  Instantiation->setInvalidDecl();
-  break;
+// Instantiate packed data members.
+if (FieldDecl *Field = dyn_cast(Member);
+Field && isa(Field->getType().getTypePtr())) {
+  QualType PatternType = Field->getType()
+ ->castAs()
+ ->getPattern();
+  std::optional NumArgumentsInExpansion =
+  getNumArgumentsInExpansion(Field->getType(), TemplateArgs);
+  assert(NumArgumentsInExpansion && "should not see unknown template argument here");
+  for (unsigned Arg = 0; Arg < *NumArgumentsInExpansion; ++Arg) {
+// Generate a new field from PackExpansion field.
+if (Decl *NewMember = Instantiator.Visit(Member)) {
+  FieldDecl *PackedField = cast(NewMember);
+  Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(*this, Arg);
+  QualType T =
+  SubstType(PatternType, TemplateArgs, PackedField->getLocation(),
+PackedField->getDeclName());
+  PackedField->setType(T);
+  Fields.push_back(PackedField);
+  if (NewMember->isInvalidDecl())
+Instantiation->setInvalidDecl();
+} else {
+  // FIXME: Eventually, a NULL return will mean that one of the
+  // instantiations was a semantic disaster, and we'll want to mark
+  // the declaration invali

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao marked an inline comment as done.
SlaterLatiao added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3292-3295
+  // FIXME: Eventually, a NULL return will mean that one of the
+  // instantiations was a semantic disaster, and we'll want to mark
+  // the declaration invalid. For now, we expect to skip some members
+  // that we can't yet handle.

dblaikie wrote:
> Worth having a test case showing that/what's broken here?
It's copied from the handling of non-pack members and possibly a duplicate that 
should be avoided. I'm not sure which case could trigger this branch. Will look 
into it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

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

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:5930-5931
   break;
+case DeclaratorContext::Member:
+  // Expand for packed data members.
 case DeclaratorContext::TemplateParam:

cjdb wrote:
> dblaikie wrote:
> > Perhaps a few more words here would be good - quoting a WG21 proposal paper 
> > that this code implements, similar to the standard quotations in nearby 
> > code. (be good to mention the WG21 proposal paper/whatnot in the patch 
> > description/commit message too)
> I would suggest a link to the discourse discussion instead of [[ 
> http://wg21.link/P1858 | P1858]], since that thread talks about the proposal 
> and goes into a bit more detail than the [[ 
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p1858r2.html#member-packs
>  | member packs ]] section of the proposal.
@dblaikie  This does not implement a C++ proposal - especially one that would 
be anywhere near approval (ie we had vague talk that WG21 want something in 
that space in P1858 and related papers, but no actual specification or 
consensus of any kind) and this PR also does not implement or have proposals 
for all  the complications this features has in terms of accessing such member 
packs
(ie packs in non dependent context, disambiguation between member of a variadic 
variable and pack members and combinations of the two)

This is a very interesting experiment, but we should be careful not to approve 
an extension that is going to conflict with future C++ language features


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

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

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Christopher Di Bella via Phabricator via cfe-commits]

cjdb added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3270-3339
+// Instantiate packed data members.
+if (FieldDecl *Field = dyn_cast(Member);
+Field && isa(Field->getType().getTypePtr())) {
+  QualType PatternType = Field->getType()
+ ->castAs()
+ ->getPattern();
+  std::optional NumArgumentsInExpansion =

In order to help improve the readability of this function, I suggest breaking 
the branches into two separate functions.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

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

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao updated this revision to Diff 546257.
SlaterLatiao added a comment.

- Address review comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGenCXX/data_member_packs.cpp

Index: clang/test/CodeGenCXX/data_member_packs.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/data_member_packs.cpp
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 --std=c++20 %s -emit-llvm -o - -triple x86_64-linux | FileCheck %s --check-prefixes=CHECK
+
+// Tests declaration data member packs.
+template struct S1 {
+Ts... ts;
+};
+
+template struct S2 {
+T t[2];
+Ts... ts;
+};
+
+// CHECK: %struct.S1 = type { i32 }
+S1 s1;
+// CHECK-NEXT: %struct.S1.0 = type { i32, float, double }
+S1 s2;
+// Test template args as the last arg.
+// CHECK-NEXT: %struct.S2 = type { [2 x i32], float, double }
+S2 s3;
+// Test nested template args.
+// CHECK-NEXT: %struct.S1.1 = type { i32, float, %struct.S1.2 }
+// CHECK-NEXT: %struct.S1.2 = type { double, double }
+S1> s4;
+// Test empty template arg.
+// CHECK-NEXT: %struct.S1.3 = type { i8 }
+S1<> s5;
+// Test duplicate types in template args.
+// CHECK-NEXT: %struct.S1.4 = type { i32, i32 }
+S1 s6;
+
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -5927,6 +5927,9 @@
  /*ExpectPackInType=*/false);
   }
   break;
+case DeclaratorContext::Member:
+  // Expand for data member packs.
+  // https://discourse.llvm.org/t/adding-support-for-data-member-packs/71333
 case DeclaratorContext::TemplateParam:
   // C++0x [temp.param]p15:
   //   If a template-parameter is a [...] is a parameter-declaration that
@@ -5954,7 +5957,6 @@
 case DeclaratorContext::CXXNew:
 case DeclaratorContext::AliasDecl:
 case DeclaratorContext::AliasTemplate:
-case DeclaratorContext::Member:
 case DeclaratorContext::Block:
 case DeclaratorContext::ForInit:
 case DeclaratorContext::SelectionInit:
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3267,42 +3267,81 @@
   continue;
 }
 
-Decl *NewMember = Instantiator.Visit(Member);
-if (NewMember) {
-  if (FieldDecl *Field = dyn_cast(NewMember)) {
-Fields.push_back(Field);
-  } else if (EnumDecl *Enum = dyn_cast(NewMember)) {
-// C++11 [temp.inst]p1: The implicit instantiation of a class template
-// specialization causes the implicit instantiation of the definitions
-// of unscoped member enumerations.
-// Record a point of instantiation for this implicit instantiation.
-if (TSK == TSK_ImplicitInstantiation && !Enum->isScoped() &&
-Enum->isCompleteDefinition()) {
-  MemberSpecializationInfo *MSInfo =Enum->getMemberSpecializationInfo();
-  assert(MSInfo && "no spec info for member enum specialization");
-  MSInfo->setTemplateSpecializationKind(TSK_ImplicitInstantiation);
-  MSInfo->setPointOfInstantiation(PointOfInstantiation);
-}
-  } else if (StaticAssertDecl *SA = dyn_cast(NewMember)) {
-if (SA->isFailed()) {
-  // A static_assert failed. Bail out; instantiating this
-  // class is probably not meaningful.
-  Instantiation->setInvalidDecl();
-  break;
+// Instantiate packed data members.
+if (FieldDecl *Field = dyn_cast(Member);
+Field && isa(Field->getType().getTypePtr())) {
+  QualType PatternType = Field->getType()
+ ->castAs()
+ ->getPattern();
+  std::optional NumArgumentsInExpansion =
+  getNumArgumentsInExpansion(Field->getType(), TemplateArgs);
+  assert(NumArgumentsInExpansion && "should not see unknown template argument here");
+  for (unsigned Arg = 0; Arg < *NumArgumentsInExpansion; ++Arg) {
+// Generate a new field from PackExpansion field.
+if (Decl *NewMember = Instantiator.Visit(Member)) {
+  FieldDecl *PackedField = cast(NewMember);
+  Sema::ArgumentPackSubstitutionIndexRAII SubstIndex(*this, Arg);
+  QualType T =
+  SubstType(PatternType, TemplateArgs, PackedField->getLocation(),
+PackedField->getDeclName());
+  PackedField->setType(T);
+  Fields.push_back(PackedField);
+  if (NewMember->isInvalidDecl()) {
+// When `NewMember` has type of `PackExpansionType`, it escapes
+// validation checks in `Visit`. Handling of such cases
+

[PATCH] D156546: [Clang][WIP]Experimental implementation of data member packs declarations

[Zenong Zhang via Phabricator via cfe-commits]

SlaterLatiao marked an inline comment as done.
SlaterLatiao added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3289-3290
+  Fields.push_back(PackedField);
+  if (NewMember->isInvalidDecl())
+Instantiation->setInvalidDecl();
+} else {

dblaikie wrote:
> Is this codepath tested?
Some checks in `Visit` need to be implemented for this branch. I elaborated in 
the comment and will implement in a future differential.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:3292-3295
+  // FIXME: Eventually, a NULL return will mean that one of the
+  // instantiations was a semantic disaster, and we'll want to mark
+  // the declaration invalid. For now, we expect to skip some members
+  // that we can't yet handle.

SlaterLatiao wrote:
> dblaikie wrote:
> > Worth having a test case showing that/what's broken here?
> It's copied from the handling of non-pack members and possibly a duplicate 
> that should be avoided. I'm not sure which case could trigger this branch. 
> Will look into it.
We Traced back and could not find a test case associated with this block.  I 
added a cross reference of this to the FIXME below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156546/new/

https://reviews.llvm.org/D156546

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