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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added inline comments.


================
Comment at: libcxx/include/utility:1631
 
+#if _LIBCPP_STD_VER > 20
+template <class _Tp>
----------------
Would it be possible to make it available for earlier versions? I like to use this in D97115? I'm aware we're reluctant to offer features to earlier versions.


================
Comment at: libcxx/include/utility:1633
+template <class _Tp>
+inline _LIBCPP_INLINE_VISIBILITY constexpr underlying_type_t<_Tp>
+to_underlying(_Tp __val) noexcept {
----------------
The `inline` isn't needed.


================
Comment at: libcxx/test/std/utilities/utility/utility.underlying/to_underlying.pass.cpp:23
+enum class e_ushort : unsigned short { d = 10, e = 100, f = 200 };
+enum class e_longlong : long long {
+  low = std::numeric_limits<long long>::min(),
----------------
Maybe interesting to also test these bounds for an plain `enum`.


================
Comment at: libcxx/test/std/utilities/utility/utility.underlying/to_underlying.pass.cpp:27
+};
+enum e_non_class { enum_a = 10, enum_b = 11, enum_c = 12 };
+
----------------
This isn't used in the tests below.


================
Comment at: libcxx/utils/generate_feature_test_macro_components.py:599
+    "name": "__cpp_lib_to_underlying",
+    "values": { "c++2b": 202102 },
+    "headers": ["utility"],
----------------
The paper has no number. Is it certain this will be the final value?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97365