Make underlying_type SFINAE-friendly

[zoecarver]

Summary

I couldn't view the the document (P0340R3). But I think this implementation should work.

[mclow.lists]

Here's the whole wording (from P0340):

template <class T>
struct underlying_type;

If T is an enumeration type, the member typedef type names the underlying type of T (9.6 [dcl.enum]); otherwise, there is no member type.

Mandates: T shall be a is not an incomplete enumeration type. (9.6)

So you'll need a test of an incomplete enumeration type.

include/type_traits

4702–4703

I think I'd rather bury the boolean in an __impl class.

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

66

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.

[EricWF]

include/type_traits

4702–4703

+1. We shouldn't change the top-level declaration like this. It's technically an ABI break.

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

65

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.

66

Why not both? Passing tests are nice too.

[zoecarver]

include/type_traits

4702–4703

Will do.

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

66

The failing test error would be

no matching function for call to 'test_sfinae'

There would be a note:

candidate template ignored: substitution failure [with T = int]: no type named 'type' in 'std::__1::underlying_type<int>'

Is there any way to test for the note too?

I will add both.

[zoecarver]

Add fail test, update pass test to work with any version, use __impl_....

[EricWF]

include/type_traits

4713

Nit. Space between underlying_type and :.

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.fail.cpp

23

No need to get SFINAE involved here. Simply write:

int main() {
  using T = std::underlying_type<int>::type; // expected-error {{boom}}
}

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

30

I'm not sure what this test is actually testing. How about:

template <class T, class = typename std::underlying_type<T>::type>
std::true_type test_sfinae(int);
template <class T>
std::false_type test_sfinae(...);

int main() {
  static_assert(decltype(test_sfinae<int>(0))::value == false, "not an enum");
   static_assert(decltype(test_sfinae<E>(0))::value == true, "is an enum");

}

[mclow.lists]

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

30

Now that we have the failing test, do we need this test at all?

[zoecarver]

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.fail.cpp

23

Yes, your right. That's what happens when you copy and paste.

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

30

I like that idea @EricWF.

Thinking more about how to structure these tests, I think the "fail" test is not super helpful (as is). What this should be testing for is whether passing underlying_type a non-enum type breaks the compiler. Given that it _shouldn't_ break the compiler, I think it is more helpful to make sure it passes than to try and make sure it fails in the "right" way. Let me know if you disagree though.

[mclow.lists]

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

30

I don't know what you mean by "breaks the compiler" here.

[zoecarver]

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

30

I mean before this patch, the below code could not compile. The compiler would error when it tried to interpret the underlying_type template:

template<class T>
std::enable_if_t<
std::is_same_v<std::underlying_type_t<T>, int>
> test_sfinae(T t);

void test_sfinae(int) { return; }

int main()
{
	test_sfinae(0); // Error while substituting deduced template arguments into function template 'test_sfinae' [with T = int]
}

[mclow.lists]

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

30

And you believe that the compiler was wrong to do so?
If so, have you filed a bug against clang?

[mclow.lists]

You should probably have at least one (and probably several) simple tests that underlying_type<non enum type> is a valid type.

using Int = std::underlying_type<int>;
using Float = std::underlying_type<float>;

etc.

[zoecarver]

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

30

Sorry, to clarify, what I meant by "breaks the compiler" was that there was an error while compiling. I did not mean that clang was broken in any way or did anything wrong. I should have chosen my wording more carefully.

[zoecarver]

Update tests (tentatively) & fix stylistic error.

[EricWF]

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

37

This test runs in C++03 mode. Which is weird because it can use a lot of C++11 features; but constexpr isn't one of them.

In C++11 this is a great improvement over my messy test. But the mess is needed for C++03.

[EricWF]

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.fail.cpp

9

Add // UNSUPPORTED: c++98, c++03 here to disable the test before 11.
The other test should be sufficient to cover C++03.

[zoecarver]

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

37

I thought about that but IIRC underlying_type wasn't added until 2011. I can update if needed though.

[zoecarver]

Add UNSUPPORTED comment.

[zoecarver]

Move comment to correct file.

[jloser]

We should mark P0340R3 as complete with this patch. This is done in libcxx/www/cxx2a_status.html.

[zoecarver]

Update status.

[zoecarver]

Thanks @jloser. I always forget to do that :P

[mclow.lists]

I forgot about this patch, and went off and implemented this myself. Sorry about that.

Reviewed as D63574; committed as r364094.

include/type_traits

 
 template <class _Tp>
 _LIBCPP_INLINE_VAR constexpr bool is_nothrow_swappable_v
     = is_nothrow_swappable<_Tp>::value;
 
 #endif // _LIBCPP_STD_VER > 14
 
 #ifdef _LIBCPP_UNDERLYING_TYPE
 
-template <class _Tp>
-struct underlying_type
+template <class _Tp, bool = is_enum<_Tp>::value>
+struct __impl_underlying_type
 {
     typedef _LIBCPP_UNDERLYING_TYPE(_Tp) type;
 };
 
+template <class _Tp>
+struct __impl_underlying_type<_Tp, false> { };
+
+template <class _Tp>
+struct underlying_type : __impl_underlying_type<_Tp> { };
+
 #if _LIBCPP_STD_VER > 11
 template <class _Tp> using underlying_type_t = typename underlying_type<_Tp>::type;
 #endif
 
 #else  // _LIBCPP_UNDERLYING_TYPE
 
 template <class _Tp, bool _Support = false>
 struct underlying_type

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.fail.cpp

+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// UNSUPPORTED: c++98, c++03
+
+// type_traits
+
+// underlying_type
+
+#include <type_traits>
+
+int main(int, char**)
+{
+  // no `type` member indicates that `int` is not an enum
+  using t = std::underlying_type<int>::type; // expected-error {{no type named 'type' in 'std::__1::underlying_type<int>'}}
+  return 0;
+}

test/std/utilities/meta/meta.trans/meta.trans.other/underlying_type.pass.cpp

 #else
     #define TEST_UNSIGNED_UNDERLYING_TYPE 0 // MSVC's ABI doesn't follow the Standard
 #endif
 
 #if TEST_UNSIGNED_UNDERLYING_TYPE
 enum F { W = UINT_MAX };
 #endif // TEST_UNSIGNED_UNDERLYING_TYPE
 
+template <class T, class = typename std::underlying_type<T>::type>
+std::true_type test_sfinae(int);
+
+template <class T>
+std::false_type test_sfinae(...);
+
+template <class T>
+constexpr bool test_sfinae_v() // This is a function so it is valid pre-c++14
+{
+  return decltype(test_sfinae<T>(0))::value;
+}
+
 int main(int, char**)
 {
     ASSERT_SAME_TYPE(int, std::underlying_type<E>::type);
 #if TEST_UNSIGNED_UNDERLYING_TYPE
     ASSERT_SAME_TYPE(unsigned, std::underlying_type<F>::type);
 #endif // TEST_UNSIGNED_UNDERLYING_TYPE
 
 #if TEST_STD_VER > 11
     ASSERT_SAME_TYPE(int, std::underlying_type_t<E>);
 #if TEST_UNSIGNED_UNDERLYING_TYPE
     ASSERT_SAME_TYPE(unsigned, std::underlying_type_t<F>);
 #endif // TEST_UNSIGNED_UNDERLYING_TYPE
 #endif // TEST_STD_VER > 11
 
 #if TEST_STD_VER >= 11
     enum G : char { };
 
     ASSERT_SAME_TYPE(char,   std::underlying_type<G>::type);
 #if TEST_STD_VER > 11
     ASSERT_SAME_TYPE(char, std::underlying_type_t<G>);
 #endif // TEST_STD_VER > 11
 #endif // TEST_STD_VER >= 11
   
+  static_assert(!test_sfinae_v<int>(), "Not an enum");
+  static_assert(test_sfinae_v<E>(), "Is an enum");
+
   return 0;
 }

www/cxx2a_status.html

-->
   <tr><td><a href="https://wg21.link/P1210R0">P1210R0</a></td><td>LWG</td><td>Completing the Rebase of Library Fundamentals, Version 3, Working Draft</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P1236R1">P1236R1</a></td><td>CWG</td><td>Alternative Wording for P0907R4 Signed Integers are Two's Complement</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P1248R1">P1248R1</a></td><td>LWG</td><td>Remove CommonReference requirement from StrictWeakOrdering (a.k.a Fixing Relations)</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P1285R0">P1285R0</a></td><td>LWG</td><td>Improving Completeness Requirements for Type Traits</td><td>San Diego</td><td><i> </i></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P1353R0">P1353R0</a></td><td>CWG</td><td>Missing feature test macros</td><td>San Diego</td><td><i> </i></td><td></td></tr>
 
    <tr><td></td><td></td><td></td><td></td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0339R6">P0339R6</a></td><td>LWG</td><td>polymorphic_allocator<> as a vocabulary type</td><td>Kona</td><td></td><td></td></tr>
-  <tr><td><a href="https://wg21.link/P0340R3">P0340R3</a></td><td>LWG</td><td>Making std::underlying_type SFINAE-friendly</td><td>Kona</td><td></td><td></td></tr>
+  <tr><td><a href="https://wg21.link/P0340R3">P0340R3</a></td><td>LWG</td><td>Making std::underlying_type SFINAE-friendly</td><td>Kona</td><td>Complete</td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0738R2">P0738R2</a></td><td>LWG</td><td>I Stream, You Stream, We All Stream for istream_iterator</td><td>Kona</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0811R3">P0811R3</a></td><td>LWG</td><td>Well-behaved interpolation for numbers and pointers</td><td>Kona</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P0920R2">P0920R2</a></td><td>LWG</td><td>Precalculated hash values in lookup</td><td>Kona</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P1001R2">P1001R2</a></td><td>LWG</td><td>Target Vectorization Policies from Parallelism V2 TS to C++20</td><td>Kona</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P1024R3">P1024R3</a></td><td>LWG</td><td>Usability Enhancements for std::span</td><td>Kona</td><td>Complete</td><td>9.0</td></tr>
   <tr><td><a href="https://wg21.link/P1164R1">P1164R1</a></td><td>LWG</td><td>Make create_directory() Intuitive</td><td>Kona</td><td></td><td></td></tr>
   <tr><td><a href="https://wg21.link/P1227R2">P1227R2</a></td><td>LWG</td><td>Signed ssize() functions, unsigned size() functions </td><td>Kona</td><td>Complete</td><td>9.0</td></tr>
   <tr><td><a href="https://wg21.link/P1252R2">P1252R2</a></td><td>LWG</td><td>Ranges Design Cleanup</td><td>Kona</td><td></td><td></td></tr>