[libcxx-commits] [PATCH] D97365: [libc++] [C++2b] [P1682] Add to_underlying.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/utility:1635
+__to_underlying(_Tp __val) noexcept {
+  return static_cast<underlying_type_t<_Tp> >(__val);
+}
----------------
Mordante wrote:
> Since we also use bitmask types in C++11, would it be possible to make this available in C++11? `std::underlying_type` is available in C++11.
Your friendly neighborhood ADL-hater says: `_VSTD::__to_underlying`


================
Comment at: libcxx/test/std/utilities/utility/utility.underlying/to_underlying.fail.cpp:9
+
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++2a
+
----------------
curdeius wrote:
> zoecarver wrote:
> > Are we going to retire the `c++2a` alias at some point? Maybe this should be `c++20` instead. 
> Hmm, when all compilers we support will accept `-std=c++20`?
> IIUC, that's because lit std param is verified by injecting into `-std=...`, so we can't just use `c++20` on a compiler that only accepts `-std=c++2a`.
> Maybe there's some clever hack on lit params to translate c++20 into c++2a if the former isn't supported.
> But I haven't looked into it.
My (untested) impression is that
- we cannot change only this `c++2a` to `c++20`, because this needs to exactly match llvm-lit's name `--param std=c++2a`
- we SHOULD change llvm-lit to use `--param std=c++20`, AND to add `--param std=c++2b` at the same time; but this change is above my pay grade
- one reason that change is difficult is because llvm-lit's `--param std=c++XX` relates, in some (non-obvious-to-me) way, to whether the actual compiler supports a `-std=c++XX` option. That entanglement SHOULD NOT exist, but it does. (We SHOULD just teach llvm-lit to map `std=c++20` onto `-std=c++{20,2a}` depending on compiler support.)
- and maybe the above change would require re-configuring or re-deploying buildkite and/or other infrastructure in ways that a simple "git push" wouldn't accomplish. I don't know and am scared to assume.


================
Comment at: libcxx/test/std/utilities/utility/utility.underlying/to_underlying.pass.cpp:61
+
+  return true;
+}
----------------
I would be interested to see how `std::to_underlying(s.bf)` works on an enum bit-field. I know that's more of a core-language-trivia issue than a library-trivia issue, but it'd still be nice to have a test case.

And if you can construct a test case for the ADL issue (maybe involving `enum X<Holder<Incomplete>>::E`? I'd have to look up the rules again), that'd be cool (but not a blocker).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97365