Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Eric Fiselier via cfe-commits]

EricWF added a subscriber: EricWF.
EricWF added a comment.

Ping? Is there a reason this hasn't been committed?


http://reviews.llvm.org/D15421



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

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Louis Dionne via cfe-commits]

ldionne updated this revision to Diff 58619.
ldionne added a comment.

Rebase the patch on top of the current `master`. The patch passes all of 
Clang's tests (`make check-clang`). I also removed the TODO comment about 
diagnostics, since that was answered by Richard Smith.


http://reviews.llvm.org/D15421

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/DeclTemplate.h
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/DeclTemplate.cpp
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/PCH/type_pack_element.cpp
  test/SemaCXX/type_pack_element.cpp

Index: test/SemaCXX/type_pack_element.cpp
===
--- /dev/null
+++ test/SemaCXX/type_pack_element.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+static_assert(__has_builtin(__type_pack_element), "");
+
+using SizeT = decltype(sizeof(int));
+
+template 
+using TypePackElement = __type_pack_element;
+
+template 
+struct X;
+
+static_assert(__is_same(TypePackElement<0, X<0>>, X<0>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>>, X<1>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>, X<2>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>, X<2>>, X<1>), "");
+static_assert(__is_same(TypePackElement<2, X<0>, X<1>, X<2>>, X<2>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>, X<2>, X<3>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>, X<2>, X<3>>, X<1>), "");
+static_assert(__is_same(TypePackElement<2, X<0>, X<1>, X<2>, X<3>>, X<2>), "");
+static_assert(__is_same(TypePackElement<3, X<0>, X<1>, X<2>, X<3>>, X<3>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>, X<2>, X<3>, X<4>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>, X<2>, X<3>, X<4>>, X<1>), "");
+static_assert(__is_same(TypePackElement<2, X<0>, X<1>, X<2>, X<3>, X<4>>, X<2>), "");
+static_assert(__is_same(TypePackElement<3, X<0>, X<1>, X<2>, X<3>, X<4>>, X<3>), "");
+static_assert(__is_same(TypePackElement<4, X<0>, X<1>, X<2>, X<3>, X<4>>, X<4>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<1>), "");
+static_assert(__is_same(TypePackElement<2, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<2>), "");
+static_assert(__is_same(TypePackElement<3, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<3>), "");
+static_assert(__is_same(TypePackElement<4, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<4>), "");
+static_assert(__is_same(TypePackElement<5, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<5>), "");
+
+// Test __type_pack_element with more than 2 top-level template arguments.
+static_assert(__is_same(__type_pack_element<5, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<5>), "");
+
+template 
+using ErrorTypePackElement1 = __type_pack_element; // expected-error{{may not be accessed at an out of bounds index}}
+using illformed1 = ErrorTypePackElement1<3, X<0>, X<1>>;  // expected-note{{in instantiation}}
Index: test/PCH/type_pack_element.cpp
===
--- /dev/null
+++ test/PCH/type_pack_element.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
+template 
+struct X { };
+
+using SizeT = decltype(sizeof(int));
+
+template 
+using TypePackElement = __type_pack_element;
+
+void fn1() {
+  X<0> x0 = TypePackElement<0, X<0>, X<1>, X<2>>{};
+  X<1> x1 = TypePackElement<1, X<0>, X<1>, X<2>>{};
+  X<2> x2 = TypePackElement<2, X<0>, X<1>, X<2>>{};
+}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -4191,6 +4191,8 @@
  PREDEF_DECL_CF_CONSTANT_STRING_ID);
   RegisterPredefDecl(Context.CFConstantStringTagDecl,
  PREDEF_DECL_CF_CONSTANT_STRING_TAG_ID);
+  RegisterPredefDecl(Context.TypePackElementDecl,
+ PREDEF_DECL_TYPE_PACK_ELEMENT_ID);
 
   // Build a record containing all of the tentative definitions in this file, in
   // TentativeDefinitions order.  Generally, this record will be empty for
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6428,6 +6428,9 @@
 
   case PREDEF_DECL_CF_CONSTANT_STRING_TAG_ID:
 return Context.getCFConstantStringTagDecl();
+
+  case PREDEF_DECL_TYPE_PACK_ELEMENT_ID:
+

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Louis Dionne via cfe-commits]

ldionne added a comment.

No problem for the delay; we're all busy :-). Unfortunately, the patch does not 
apply cleanly on `master` anymore, so I'll rebase it and someone can then 
commit it for me.


http://reviews.llvm.org/D15421



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

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Richard Smith via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Sorry for the delay, this all LGTM. Do you need someone to commit it for you?



Comment at: lib/Sema/SemaTemplate.cpp:2082-2085
@@ +2081,6 @@
+// If the Index is out of bounds, the program is ill-formed.
+//
+// TODO:
+// This is not actually mandated by the standard, of course, so does it
+// have its place here?
+TemplateArgument IndexArg = Converted[0], Ts = Converted[1];

This is fine; we get to specify how our extensions work, so we can say that an 
`Index` that's out of bounds is ill-formed.


http://reviews.llvm.org/D15421



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

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Louis Dionne via cfe-commits]

ldionne added a comment.

ping


http://reviews.llvm.org/D15421



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

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Louis Dionne via cfe-commits]

ldionne updated this revision to Diff 48012.
ldionne added a comment.

Address some remaining issues:

- Rename __nth_element to __type_pack_element, as suggested by Richard Smith
- Fix template parameter position issue in createTypePackElementParameterList

AFAICT, the only remaining issue is whether we should produce a diagnostic when 
the index is out of bounds, or whether this is reserved for standard 
diagnostics and we should use another mean to report the error. This is a 
trivial issue and I'll fix it or remove the TODO as soon as someone tells me 
the right way to go.

I'm really hoping that we can move forward with this patch.


http://reviews.llvm.org/D15421

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/DeclTemplate.h
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/DeclTemplate.cpp
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/PCH/type_pack_element.cpp
  test/SemaCXX/type_pack_element.cpp

Index: test/SemaCXX/type_pack_element.cpp
===
--- /dev/null
+++ test/SemaCXX/type_pack_element.cpp
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+static_assert(__has_builtin(__type_pack_element), "");
+
+using SizeT = decltype(sizeof(int));
+
+template 
+using TypePackElement = __type_pack_element;
+
+template 
+struct X;
+
+static_assert(__is_same(TypePackElement<0, X<0>>, X<0>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>>, X<1>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>, X<2>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>, X<2>>, X<1>), "");
+static_assert(__is_same(TypePackElement<2, X<0>, X<1>, X<2>>, X<2>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>, X<2>, X<3>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>, X<2>, X<3>>, X<1>), "");
+static_assert(__is_same(TypePackElement<2, X<0>, X<1>, X<2>, X<3>>, X<2>), "");
+static_assert(__is_same(TypePackElement<3, X<0>, X<1>, X<2>, X<3>>, X<3>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>, X<2>, X<3>, X<4>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>, X<2>, X<3>, X<4>>, X<1>), "");
+static_assert(__is_same(TypePackElement<2, X<0>, X<1>, X<2>, X<3>, X<4>>, X<2>), "");
+static_assert(__is_same(TypePackElement<3, X<0>, X<1>, X<2>, X<3>, X<4>>, X<3>), "");
+static_assert(__is_same(TypePackElement<4, X<0>, X<1>, X<2>, X<3>, X<4>>, X<4>), "");
+
+static_assert(__is_same(TypePackElement<0, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<0>), "");
+static_assert(__is_same(TypePackElement<1, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<1>), "");
+static_assert(__is_same(TypePackElement<2, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<2>), "");
+static_assert(__is_same(TypePackElement<3, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<3>), "");
+static_assert(__is_same(TypePackElement<4, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<4>), "");
+static_assert(__is_same(TypePackElement<5, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<5>), "");
+
+// Test __type_pack_element with more than 2 top-level template arguments.
+static_assert(__is_same(__type_pack_element<5, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<5>), "");
+
+template 
+using ErrorTypePackElement1 = __type_pack_element; // expected-error{{may not be accessed at an out of bounds index}}
+using illformed1 = ErrorTypePackElement1<3, X<0>, X<1>>;  // expected-note{{in instantiation}}
Index: test/PCH/type_pack_element.cpp
===
--- /dev/null
+++ test/PCH/type_pack_element.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
+template 
+struct X { };
+
+using SizeT = decltype(sizeof(int));
+
+template 
+using TypePackElement = __type_pack_element;
+
+void fn1() {
+  X<0> x0 = TypePackElement<0, X<0>, X<1>, X<2>>{};
+  X<1> x1 = TypePackElement<1, X<0>, X<1>, X<2>>{};
+  X<2> x2 = TypePackElement<2, X<0>, X<1>, X<2>>{};
+}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -4182,6 +4182,8 @@
  PREDEF_DECL_CF_CONSTANT_STRING_ID);
   RegisterPredefDecl(Context.CFConstantStringTagDecl,
  PREDEF_DECL_CF_CONSTANT_STRING_TAG_ID);
+  RegisterPredefDecl(Context.TypePackElementDecl, 
+ PREDEF_DECL_TYPE_PACK_ELEMENT_ID);
 
   // Build a record containing all of the tentative definitions in this file, in
   // TentativeDefinitions order.  Generally, this record 

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Louis Dionne via cfe-commits]

ldionne added a reviewer: rsmith.
ldionne updated this revision to Diff 44779.
ldionne added a comment.

Rebase on top of the master branch, and add Richard Smith as a reviewer, since 
he also reviewed David Majnemer's original patch (suggested by Nathan Wilson).


http://reviews.llvm.org/D15421

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/DeclTemplate.h
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/DeclTemplate.cpp
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/PCH/nth-element.cpp
  test/SemaCXX/nth_element.cpp

Index: test/SemaCXX/nth_element.cpp
===
--- /dev/null
+++ test/SemaCXX/nth_element.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+static_assert(__has_builtin(__nth_element), "");
+
+template 
+using NthElement = __nth_element;
+
+template 
+struct X;
+
+static_assert(__is_same(NthElement<0, X<0>>, X<0>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>>, X<1>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>>, X<2>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>>, X<2>), "");
+static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>>, X<3>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>, X<4>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>, X<4>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>, X<4>>, X<2>), "");
+static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>, X<4>>, X<3>), "");
+static_assert(__is_same(NthElement<4, X<0>, X<1>, X<2>, X<3>, X<4>>, X<4>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<2>), "");
+static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<3>), "");
+static_assert(__is_same(NthElement<4, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<4>), "");
+static_assert(__is_same(NthElement<5, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<5>), "");
+
+template 
+using ErrorNthElement1 = __nth_element; // expected-error{{may not be accessed at an out of bounds index}}
+using illformed1 = ErrorNthElement1, X<1>>; // expected-note{{in instantiation}}
+
+
+template 
+using ErrorNthElement2 = __nth_element; // expected-error{{may not be accessed at an out of bounds index}}
+using illformed2 = ErrorNthElement2, X<1>>; // expected-note{{in instantiation}}
+
+
+template 
+using ErrorNthElement3 = __nth_element; // expected-error{{must be indexed using an integral type}}
+enum Color : int { Red, Green, Blue };
+using illformed3 = ErrorNthElement3, X<1>, X<2>>; // expected-note{{in instantiation}}
Index: test/PCH/nth-element.cpp
===
--- /dev/null
+++ test/PCH/nth-element.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
+template 
+struct X { };
+
+template 
+using NthElement = __nth_element;
+
+void fn1() {
+  X<0> x0 = NthElement<0, X<0>, X<1>, X<2>>{};
+  X<1> x1 = NthElement<1, X<0>, X<1>, X<2>>{};
+  X<2> x2 = NthElement<2, X<0>, X<1>, X<2>>{};
+}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -4152,6 +4152,7 @@
   RegisterPredefDecl(Context.ExternCContext, PREDEF_DECL_EXTERN_C_CONTEXT_ID);
   RegisterPredefDecl(Context.MakeIntegerSeqDecl,
  PREDEF_DECL_MAKE_INTEGER_SEQ_ID);
+  RegisterPredefDecl(Context.NthElementDecl, PREDEF_DECL_NTH_ELEMENT_ID);
 
   // Build a record containing all of the tentative definitions in this file, in
   // TentativeDefinitions order.  Generally, this record will be empty for
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6444,6 +6444,9 @@
 
   case PREDEF_DECL_MAKE_INTEGER_SEQ_ID:
 return Context.getMakeIntegerSeqDecl();
+
+  case 

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Nathan Wilson via cfe-commits]

nwilson added a comment.

In http://reviews.llvm.org/D15421#326144, @rsmith wrote:

> Bikeshedding on the name a bit... how about `__type_pack_element`?


Hmm, I kind of felt like having `nth` in there implied we're indexing into 
something... What about `__nth_pack_element`?


http://reviews.llvm.org/D15421



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

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: include/clang/Basic/Builtins.h:216-219
@@ -215,3 +215,6 @@
   /// \brief This names the __make_integer_seq BuiltinTemplateDecl.
   BTK__make_integer_seq
+
+  /// \brief This names the __nth_element BuiltinTemplateDecl.
+  , BTK__nth_element
 };

Please put the comma after `BTK__make_integer_seq`, not on this line.


Comment at: lib/AST/DeclTemplate.cpp:1243
@@ +1242,3 @@
+static TemplateParameterList *
+createNthElement(const ASTContext , DeclContext *DC) {
+  // typename IndexType

Rename this `createNthElementParameterList`.


Comment at: lib/AST/DeclTemplate.cpp:1244-1248
@@ +1243,7 @@
+createNthElement(const ASTContext , DeclContext *DC) {
+  // typename IndexType
+  auto *IndexType = TemplateTypeParmDecl::Create(
+  C, DC, SourceLocation(), SourceLocation(), /*Depth=*/0, /*Position=*/0,
+  /*Id=*/nullptr, /*Typename=*/true, /*ParameterPack=*/false);
+  IndexType->setImplicit(true);
+

Use `NonTypeTemplateParmDecl::Create`. You can get `size_t` from 
`ASTContext::getSizeType`.


Comment at: lib/Sema/SemaTemplate.cpp:2102-2103
@@ +2101,4 @@
+// We simply return the type at index `Index`.
+// TODO:
+// What are the implications of calling .getExtValue() on an APSInt?
+assert(Index.getExtValue() == Index &&

`getExtValue` is OK here, as we don't support `size_t` being larger than 64 
bits.


http://reviews.llvm.org/D15421



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

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Richard Smith via cfe-commits]

rsmith added a comment.

Bikeshedding on the name a bit... how about `__type_pack_element`?


http://reviews.llvm.org/D15421



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

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Richard Smith via cfe-commits]

On Wed, Jan 13, 2016 at 2:31 PM, Nathan Wilson  wrote:

> nwilson added a comment.
>
> In http://reviews.llvm.org/D15421#326144, @rsmith wrote:
>
> > Bikeshedding on the name a bit... how about `__type_pack_element`?
>
> Hmm, I kind of felt like having `nth` in there implied we're indexing into
> something... What about `__nth_pack_element`?


Conversely, std::nth_element doesn't do indexing, and std::tuple_element
does.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Nathan Wilson via cfe-commits]

On Wed, Jan 13, 2016 at 6:48 PM, Richard Smith 
wrote:

> On Wed, Jan 13, 2016 at 3:41 PM, Arthur O'Dwyer via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Following Louis' suggestion, how about __pack_nth?
>>
>
> Maybe just __pack_element, to mirror its intended use to implement things
> like tuple_element? (I'm not completely happy about using this general name
> for something that only works for packs of types, but given that this
> template produces a type, it's probably the best we can do.)
>

That seems fine to me. I do feel like a name for something else will be
shoehorned in later as well though.

Louis - what do you think?


>
>
>> On Wed, Jan 13, 2016 at 3:16 PM, Nathan Wilson via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>>
>>>
>>> On Wed, Jan 13, 2016 at 4:52 PM, Richard Smith 
>>> wrote:
>>>
 On Wed, Jan 13, 2016 at 2:31 PM, Nathan Wilson 
 wrote:

> nwilson added a comment.
>
> In http://reviews.llvm.org/D15421#326144, @rsmith wrote:
>
> > Bikeshedding on the name a bit... how about `__type_pack_element`?
>
> Hmm, I kind of felt like having `nth` in there implied we're indexing
> into something... What about `__nth_pack_element`?


 Conversely, std::nth_element doesn't do indexing, and
 std::tuple_element does.

>>>
>>> Yeah, I was trying to combine them, but maybe that's misleading.
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Richard Smith via cfe-commits]

On Wed, Jan 13, 2016 at 3:41 PM, Arthur O'Dwyer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Following Louis' suggestion, how about __pack_nth?
>

Maybe just __pack_element, to mirror its intended use to implement things
like tuple_element? (I'm not completely happy about using this general name
for something that only works for packs of types, but given that this
template produces a type, it's probably the best we can do.)


> On Wed, Jan 13, 2016 at 3:16 PM, Nathan Wilson via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>>
>>
>> On Wed, Jan 13, 2016 at 4:52 PM, Richard Smith 
>> wrote:
>>
>>> On Wed, Jan 13, 2016 at 2:31 PM, Nathan Wilson 
>>> wrote:
>>>
 nwilson added a comment.

 In http://reviews.llvm.org/D15421#326144, @rsmith wrote:

 > Bikeshedding on the name a bit... how about `__type_pack_element`?

 Hmm, I kind of felt like having `nth` in there implied we're indexing
 into something... What about `__nth_pack_element`?
>>>
>>>
>>> Conversely, std::nth_element doesn't do indexing, and std::tuple_element
>>> does.
>>>
>>
>> Yeah, I was trying to combine them, but maybe that's misleading.
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Louis Dionne via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: majnemer.
ldionne added a subscriber: cfe-commits.

This patch adds a `__nth_element` builtin that allows fetching the n-th type of 
a parameter pack with very little compile-time overhead. The patch was inspired 
by r252036 and r252115 by David Majnemer, which add a similar 
`__make_integer_seq` builtin for efficiently creating a `std::integer_sequence`.

http://reviews.llvm.org/D15421

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/DeclTemplate.h
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/DeclTemplate.cpp
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/PCH/nth-element.cpp
  test/SemaCXX/nth_element.cpp

Index: test/SemaCXX/nth_element.cpp
===
--- /dev/null
+++ test/SemaCXX/nth_element.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+static_assert(__has_builtin(__nth_element), "");
+
+template 
+using NthElement = __nth_element;
+
+template 
+struct X;
+
+static_assert(__is_same(NthElement<0, X<0>>, X<0>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>>, X<1>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>>, X<2>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>>, X<2>), "");
+static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>>, X<3>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>, X<4>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>, X<4>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>, X<4>>, X<2>), "");
+static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>, X<4>>, X<3>), "");
+static_assert(__is_same(NthElement<4, X<0>, X<1>, X<2>, X<3>, X<4>>, X<4>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<2>), "");
+static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<3>), "");
+static_assert(__is_same(NthElement<4, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<4>), "");
+static_assert(__is_same(NthElement<5, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<5>), "");
+
+template 
+using ErrorNthElement1 = __nth_element; // expected-error{{may not be accessed at an out of bounds index}}
+using illformed1 = ErrorNthElement1, X<1>>; // expected-note{{in instantiation}}
+
+
+template 
+using ErrorNthElement2 = __nth_element; // expected-error{{may not be accessed at an out of bounds index}}
+using illformed2 = ErrorNthElement2, X<1>>; // expected-note{{in instantiation}}
+
+
+template 
+using ErrorNthElement3 = __nth_element; // expected-error{{must be indexed using an integral type}}
+enum Color : int { Red, Green, Blue };
+using illformed3 = ErrorNthElement3, X<1>, X<2>>; // expected-note{{in instantiation}}
Index: test/PCH/nth-element.cpp
===
--- /dev/null
+++ test/PCH/nth-element.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
+template 
+struct X { };
+
+template 
+using NthElement = __nth_element;
+
+void fn1() {
+  X<0> x0 = NthElement<0, X<0>, X<1>, X<2>>{};
+  X<1> x1 = NthElement<1, X<0>, X<1>, X<2>>{};
+  X<2> x2 = NthElement<2, X<0>, X<1>, X<2>>{};
+}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -4145,6 +4145,7 @@
   RegisterPredefDecl(Context.ExternCContext, PREDEF_DECL_EXTERN_C_CONTEXT_ID);
   RegisterPredefDecl(Context.MakeIntegerSeqDecl,
  PREDEF_DECL_MAKE_INTEGER_SEQ_ID);
+  RegisterPredefDecl(Context.NthElementDecl, PREDEF_DECL_NTH_ELEMENT_ID);
 
   // Build a record containing all of the tentative definitions in this file, in
   // TentativeDefinitions order.  Generally, this record will be empty for
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ 

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Nathan Wilson via cfe-commits]

Hi Louis,

It would probably be useful to get the lines of context as well by doing a
full diff as describer here: http://llvm.org/docs/Phabricator.html#id3

For example, git diff -U99 



On Thu, Dec 10, 2015 at 11:32 AM, Louis Dionne via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> ldionne created this revision.
> ldionne added a reviewer: majnemer.
> ldionne added a subscriber: cfe-commits.
>
> This patch adds a `__nth_element` builtin that allows fetching the n-th
> type of a parameter pack with very little compile-time overhead. The patch
> was inspired by r252036 and r252115 by David Majnemer, which add a similar
> `__make_integer_seq` builtin for efficiently creating a
> `std::integer_sequence`.
>
> http://reviews.llvm.org/D15421
>
> Files:
>   include/clang/AST/ASTContext.h
>   include/clang/AST/DeclTemplate.h
>   include/clang/Basic/Builtins.h
>   include/clang/Basic/DiagnosticSemaKinds.td
>   include/clang/Serialization/ASTBitCodes.h
>   lib/AST/ASTContext.cpp
>   lib/AST/DeclTemplate.cpp
>   lib/Lex/PPMacroExpansion.cpp
>   lib/Sema/SemaLookup.cpp
>   lib/Sema/SemaTemplate.cpp
>   lib/Serialization/ASTReader.cpp
>   lib/Serialization/ASTWriter.cpp
>   test/PCH/nth-element.cpp
>   test/SemaCXX/nth_element.cpp
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Louis Dionne via cfe-commits]

ldionne added a comment.

We should probably consider changing the name from `__nth_element` to something 
else, perhaps something that contains the word "pack". This is especially true 
if we are to implement other intrinsics for manipulating parameter packs, like 
slicing (as proposed by Eric Fiselier). Unless someone with a clearer view of 
the project suggests something, I would probably go with `__pack_element` (and 
then we could add `__pack_slice`, etc...). I have no strong preference.

This patch also raises the question of whether we should have a more separated 
way of adding builtin templates. My job here was very easy because I just 
copied David's revision and tweaked a couple of things here and there, but I 
could imagine it not being obvious at all otherwise. Also, and most 
importantly, if we add other builtin templates, do we want these builtins to 
"clutter" the rest of the code instead of being somewhat isolated? I don't know 
the answer to these questions, but I'd just like to point them out.



Comment at: lib/AST/DeclTemplate.cpp:1245-1249
@@ +1244,7 @@
+createNthElement(const ASTContext , DeclContext *DC) {
+  // typename IndexType
+  auto *IndexType = TemplateTypeParmDecl::Create(
+  C, DC, SourceLocation(), SourceLocation(), /*Depth=*/0, /*Position=*/0,
+  /*Id=*/nullptr, /*Typename=*/true, /*ParameterPack=*/false);
+  IndexType->setImplicit(true);
+

It would be simpler and better (IMO) to have `template ` instead of `template `, but I couldn't figure out how to create a non-type template 
parameter. Hence, I just adapted the code from David's revision for 
`__make_integer_seq`.


Comment at: lib/Sema/SemaTemplate.cpp:2101-2102
@@ +2100,4 @@
+// We simply return the type at index `Index`.
+// TODO:
+// What are the implications of calling .getExtValue() on an APSInt?
+assert(Index.getExtValue() == Index &&

More generally, what is the proper way to retrieve a non-type template argument 
as a number that I can then use as a normal `std::size_t` (or similar) in the 
code? I really just want to retrieve the index, which is given as a non-type 
template parameter to `__nth_element`, and convert it to a numeric index `n` in 
order to fetch the `n`-th element of the parameter pack.


http://reviews.llvm.org/D15421



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

Re: [PATCH] D15421: [Feature] Add a builtin for indexing into parameter packs

[Louis Dionne via cfe-commits]

ldionne updated this revision to Diff 42443.
ldionne added a comment.

Added full lines of context. Sorry for not doing it the first time around.


http://reviews.llvm.org/D15421

Files:
  include/clang/AST/ASTContext.h
  include/clang/AST/DeclTemplate.h
  include/clang/Basic/Builtins.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Serialization/ASTBitCodes.h
  lib/AST/ASTContext.cpp
  lib/AST/DeclTemplate.cpp
  lib/Lex/PPMacroExpansion.cpp
  lib/Sema/SemaLookup.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/PCH/nth-element.cpp
  test/SemaCXX/nth_element.cpp

Index: test/SemaCXX/nth_element.cpp
===
--- /dev/null
+++ test/SemaCXX/nth_element.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+static_assert(__has_builtin(__nth_element), "");
+
+template 
+using NthElement = __nth_element;
+
+template 
+struct X;
+
+static_assert(__is_same(NthElement<0, X<0>>, X<0>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>>, X<1>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>>, X<2>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>>, X<2>), "");
+static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>>, X<3>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>, X<4>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>, X<4>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>, X<4>>, X<2>), "");
+static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>, X<4>>, X<3>), "");
+static_assert(__is_same(NthElement<4, X<0>, X<1>, X<2>, X<3>, X<4>>, X<4>), "");
+
+static_assert(__is_same(NthElement<0, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<0>), "");
+static_assert(__is_same(NthElement<1, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<1>), "");
+static_assert(__is_same(NthElement<2, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<2>), "");
+static_assert(__is_same(NthElement<3, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<3>), "");
+static_assert(__is_same(NthElement<4, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<4>), "");
+static_assert(__is_same(NthElement<5, X<0>, X<1>, X<2>, X<3>, X<4>, X<5>>, X<5>), "");
+
+template 
+using ErrorNthElement1 = __nth_element; // expected-error{{may not be accessed at an out of bounds index}}
+using illformed1 = ErrorNthElement1, X<1>>; // expected-note{{in instantiation}}
+
+
+template 
+using ErrorNthElement2 = __nth_element; // expected-error{{may not be accessed at an out of bounds index}}
+using illformed2 = ErrorNthElement2, X<1>>; // expected-note{{in instantiation}}
+
+
+template 
+using ErrorNthElement3 = __nth_element; // expected-error{{must be indexed using an integral type}}
+enum Color : int { Red, Green, Blue };
+using illformed3 = ErrorNthElement3, X<1>, X<2>>; // expected-note{{in instantiation}}
Index: test/PCH/nth-element.cpp
===
--- /dev/null
+++ test/PCH/nth-element.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++14 -x c++-header %s -emit-pch -o %t.pch
+// RUN: %clang_cc1 -std=c++14 -x c++ /dev/null -include-pch %t.pch
+
+template 
+struct X { };
+
+template 
+using NthElement = __nth_element;
+
+void fn1() {
+  X<0> x0 = NthElement<0, X<0>, X<1>, X<2>>{};
+  X<1> x1 = NthElement<1, X<0>, X<1>, X<2>>{};
+  X<2> x2 = NthElement<2, X<0>, X<1>, X<2>>{};
+}
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -4136,6 +4136,7 @@
   RegisterPredefDecl(Context.ExternCContext, PREDEF_DECL_EXTERN_C_CONTEXT_ID);
   RegisterPredefDecl(Context.MakeIntegerSeqDecl,
  PREDEF_DECL_MAKE_INTEGER_SEQ_ID);
+  RegisterPredefDecl(Context.NthElementDecl, PREDEF_DECL_NTH_ELEMENT_ID);
 
   // Build a record containing all of the tentative definitions in this file, in
   // TentativeDefinitions order.  Generally, this record will be empty for
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6423,6 +6423,9 @@
 
   case PREDEF_DECL_MAKE_INTEGER_SEQ_ID:
 return Context.getMakeIntegerSeqDecl();
+
+  case PREDEF_DECL_NTH_ELEMENT_ID:
+return Context.getNthElementDecl();
   }
   llvm_unreachable("PredefinedDeclIDs unknown enum value");