[PATCH] D100672: [ADT] Add new type traits for type pack indexes

[Scott Linder via Phabricator via llvm-commits]

scott.linder added a comment.

In D100672#2696022 <https://reviews.llvm.org/D100672#2696022>, @dblaikie wrote:

> Are there (future) STL equivalents of TypesAreDistinct or FirsntIndexOfType?

There aren't, as far as I'm aware. I didn't do an exhaustive search, but at the very least I don't see anything equivalent in `<type_traits>`



================
Comment at: llvm/include/llvm/ADT/STLExtras.h:151-154
+  for (size_t I = 1; I <= sizeof...(Ts); ++I)
+    if (Matches[I])
+      return I - 1;
+  return sizeof...(Ts);
----------------
dblaikie wrote:
> Could this be replaced with:
> ```
> return llvm::find(Matches, true) - std::begin(Matches);
> ```
> or something like that?
> (similarly below in "AreTypesDistinct")?
> 
> Hmm, what's the purpose of the first element of the array being 'false', and then iterating from the second element and subtracting one from the index before returning?
> 
> Oh, that's to support the zero-length case? Fair enough. I don't mind either way, but maybe it'd be simpler/more obvious if that were done via specialization? - oh, what happens if you put the buffer value at the end rather than the beginning. In this algorithm you could put "true" at the end (and remove "false" from the start) and then it'd return 1 when sizeof...(Ts) is zero?
> 
Sure, I think `llvm::find` would need to be constexpr then? I assume that will ripple through some other things in `STLExtras.h`, so maybe I should do a pass to add `constexpr` wherever possible in `STLExtras.h`? Or in this case would just marking what I need be better?

Putting the extra element at the end and having it defined to return a size >= sizeof...(Ts) seems reasonable, especially as it is just a detail function. I'll make that change, thank you!


================
Comment at: llvm/include/llvm/ADT/STLExtras.h:163
+template <typename... Ts> static constexpr bool areTypesDistinct() {
+  constexpr size_t IndexOf[] = {0, getFirstIndexOfType<Ts, Ts...>()...};
+  for (size_t I = 1; I <= sizeof...(Ts); ++I)
----------------
dblaikie wrote:
> Any concerns about the quadratic complexity here? Any more efficient way to write this, maybe? (what's the libc++ equivalent implementation look like, I wonder?)
I couldn't think of any in a constexpr context, but there very well may be.

My hope is that this is only needed once per instantiation of a type that cares about this property. I.e. once you assert that `IntrusiveVariant<Foo, Bar, Baz>` has distinct alternative types, I believe the compiler does not need to re-check that assertion. Incidentally we previously didn't assert this at all `PointerUnion`, although it seems to be a requirement?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100672