[libcxx-commits] [PATCH] D58987: Make underlying_type SFINAE-friendly

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added inline comments.


================
Comment at: include/type_traits:4714
 
-template <class _Tp>
+template <class _Tp, bool = is_enum<_Tp>::value>
 struct underlying_type
----------------
mclow.lists wrote:
> I think I'd rather bury the boolean in an `__impl` class.
+1. We shouldn't change the top-level declaration like this. It's technically an ABI break.


================
Comment at: test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp:65
+	
+#if TEST_STD_VER > 17
+	test_sfinae(0);
----------------
We're backporting this behavior and we can probably expect others to do so as well.

Therefore the tests should run for all dialects we offer this behavior.


================
Comment at: test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp:66
+#if TEST_STD_VER > 17
+	test_sfinae(0);
+#endif // TEST_STD_VER > 17
----------------
mclow.lists wrote:
> I prefer a failing test; so that we can look at the error message: `no type named type in std::underlying_type<int>` blah blah.
Why not both? Passing tests are nice too.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D58987