[PATCH] D151952: [clang] adds `__type_pack_index` so we can get a type's parameter pack index

[Daniel Bertalan via Phabricator via cfe-commits]

BertalanD added a subscriber: jwakely.
BertalanD added a comment.

I am considering making use of this feature in a real-world project. I have a few short observations after rebasing this diff and playing with it for a bit:

- It seems to me that `__has_builtin(__type_pack_index)`  currently evaluates to false. Please include a check for the feature detection working correctly in the test.

- Has there been any coordination with GCC folks on the name/semantics of this builtin?

  GCC trunk has recently gained support for `__type_pack_element`, and on the feature request for it (PR100157 <https://gcc.gnu.org/PR100157>), @jwakely has already expressed interest in implementing the reverse of that, i.e. what this diff proposes. A slight incompatibility has managed to seep in: GCC does not allow the use of `__type_pack_element` in a position where its name would end up being manged; see the Folly bug report <https://github.com/facebook/folly/issues/1991>. While this is easy to work around (wrap it in a template), as a user, it would be inconvenient needing to add a GCC/Clang check alongside `__has_builtin` if the two compilers were to come up with incompatibly behaving implementations for this one.

- What would be the expected way of gracefully handing the queried type not being present in the list?

  Our `std::variant`-like type uses a `can_contain` constraint on its accessors to prevent asking for a type that's not in the type list. We currently achieve this by `index_of` returning `sizeof...(Ts)` as a sentinel value if no match was found. libc++'s __find_exactly_one_t <https://github.com/llvm/llvm-project/blob/72c826cf6ce9d16c512c00d6c8744292392c9116/libcxx/include/tuple#L1425-L1456> uses `(size_t)-1` for the same purpose.

  I'm targeting C++20, so `requires { __type_pack_index(T, InTypes...); }` seems to do just what I want, but `std::variant` is C++17 and `std::tuple` is even earlier than that. Of course, in earlier language modes you don't have to worry about it working in concepts, but e.g. libc++ `static_assert`s that the index is not the sentinel. Not sure about what libstdc++'s requirements would be.

  In any case, could you please test/document the expected way of recovering from the error?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151952