[PATCH] D98477: [ADT] Add IntrusiveVariant

[Whisperity via Phabricator via llvm-commits]

whisperity added a comment.

I'm not qualified with template magic and I am definitely not a person of authority on this library, so all my comments come from a generally strictly "user's perspective". I would love to have a variant type (it's surprising there never has been one in the code for so long), and I am happy I found this patch instead of trying to get into writing a similar thing myself...



================
Comment at: llvm/include/llvm/ADT/IntrusiveVariant.h:207-208
+  }
+  template <std::size_t I, typename... ArgTs>
+  UnionImpl(InPlaceIndex<I>, ArgTs &&...Args) {
+    new (&getMember(InPlaceIndex<I>{}))
----------------
There is a bit too much template "magic" to wrap my head around so quickly, but I have a question: does this implementation allow non-default-constructible alternative types? Or any chance that it forbids default-constructible ones? What's the stance on that. The examples in the leading documentation seem to indicate that all alternatives are non-default-ctorable... However, the "formal" requirements only say they must be trivially-dtorable and standard layout. What's the stance on default constructibleness?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98477