[libc++][NFC] Use is_void instead of is_same<void, ...>

[Mordante]

Can you explain what the benefit of this change is? And why the different behaviour of std::same_as and std::is_void_v is not an issue. I'm not even sure we should call this an NFC patch.

In general it would be good to add this kind of information to the commit message/review description.

[nilayvaish]

In D129716#3655911, @Mordante wrote:

Can you explain what the benefit of this change is? And why the different behaviour of std::same_as and std::is_void_v is not an issue. I'm not even sure we should call this an NFC patch.

In general it would be good to add this kind of information to the commit message/review description.

The patch is replacing std::is_same with a more specialized version std::is_void. This patch seems fine to me. Two questions:

• Why do we have the specialized version at all?
• Why does std::same_as matter here?

[Mordante]

In D129716#3656967, @nilayvaish wrote:

In D129716#3655911, @Mordante wrote:

Can you explain what the benefit of this change is? And why the different behaviour of std::same_as and std::is_void_v is not an issue. I'm not even sure we should call this an NFC patch.

In general it would be good to add this kind of information to the commit message/review description.

The patch is replacing std::is_same with a more specialized version std::is_void. This patch seems fine to me. Two questions:

std::is_void is less specialized. After the change cv-qualified void types are accepted. https://godbolt.org/z/j66MsThYs

• Why do we have the specialized version at all?

You mean std::is_void? (If yes, that's required by the Standard.)

• Why does std::same_as matter here?

Sorry I don't understand the question, can you rephrase it?

[philnik]

In D129716#3655911, @Mordante wrote:

Can you explain what the benefit of this change is? And why the different behaviour of std::same_as and std::is_void_v is not an issue. I'm not even sure we should call this an NFC patch.

In general it would be good to add this kind of information to the commit message/review description.

The benefit is that it's a bit easier to read IMO. The different behaviour isn't an issue in the first three files, because it just checks that __make_32_64_or_128_bit_t or the conditional succeeded, which indicate a failure as void. The two tests check that the return type is void. There it isn't a problem, because cv-qualifiers on return types are discarded. IOW const void func(); static_assert(std::is_same_v<void, decltype(func())>); is a correct program.

[Mordante]

In D129716#3658316, @philnik wrote:

In D129716#3655911, @Mordante wrote:

Can you explain what the benefit of this change is? And why the different behaviour of std::same_as and std::is_void_v is not an issue. I'm not even sure we should call this an NFC patch.

In general it would be good to add this kind of information to the commit message/review description.

The benefit is that it's a bit easier to read IMO. The different behaviour isn't an issue in the first three files, because it just checks that __make_32_64_or_128_bit_t or the conditional succeeded, which indicate a failure as void. The two tests check that the return type is void. There it isn't a problem, because cv-qualifiers on return types are discarded. IOW const void func(); static_assert(std::is_same_v<void, decltype(func())>); is a correct program.

For me it's the opposite; I wonder what const void would mean in that context. The code in charconv and format is written by me and I didn't use is_void_v since to me the current syntax is easier to read.

[ldionne]

libcxx/test/std/containers/container.adaptors/queue/queue.defn/emplace.pass.cpp

36

IMO is_same reads better here (and in the other two test files) since it is more symmetric with other tests for the return type, where we check is_same<queue_return_type, SOMETHING>.

libcxx/include/__compare/strong_order.h

struct __fn {
                 bool __t_is_nan = _VSTD::isnan(__t);
                 bool __u_is_nan = _VSTD::isnan(__u);
                 bool __t_is_negative = _VSTD::signbit(__t);
                 bool __u_is_negative = _VSTD::signbit(__u);
                 using _IntType = conditional_t<
                     sizeof(__t) == sizeof(int32_t), int32_t, conditional_t<
                     sizeof(__t) == sizeof(int64_t), int64_t, void>
                 >;
-                if constexpr (is_same_v<_IntType, void>) {
+                if constexpr (is_void_v<_IntType>) {
                     static_assert(sizeof(_Dp) == 0, "std::strong_order is unimplemented for this floating-point type");
                 } else if (__t_is_nan && __u_is_nan) {
                     // Order by sign bit, then by "payload bits" (we'll just use bit_cast).
                     if (__t_is_negative != __u_is_negative) {
                         return (__u_is_negative <=> __t_is_negative);
                     } else {
                         return _VSTD::bit_cast<_IntType>(__t) <=> _VSTD::bit_cast<_IntType>(__u);
                     }

libcxx/include/__format/formatter_integer.h

public:
   template <integral _Tp>
   _LIBCPP_HIDE_FROM_ABI auto format(_Tp __value, auto& __ctx) const -> decltype(__ctx.out()) {
     __format_spec::__parsed_specifications<_CharT> __specs = __parser_.__get_parsed_std_specifications(__ctx);
 
     if (__specs.__std_.__type_ == __format_spec::__type::__char)
       return __formatter::__format_char(__value, __ctx.out(), __specs);
 
     using _Type = __make_32_64_or_128_bit_t<_Tp>;
-    static_assert(!is_same<_Type, void>::value, "unsupported integral type used in __formatter_integer::__format");
+    static_assert(!is_void_v<_Type>, "unsupported integral type used in __formatter_integer::__format");
 
     // Reduce the number of instantiation of the integer formatter
     return __formatter::__format_integer(static_cast<_Type>(__value), __ctx, __specs);
   }
 
   __format_spec::__parser<_CharT> __parser_;
 };
 

libcxx/include/charconv

__to_chars_integral(char* __first, char* __last, _Tp __value, int __base,
   return {__last, errc(0)};
 }
 
 template <typename _Tp, typename enable_if<is_integral<_Tp>::value, int>::type = 0>
 inline _LIBCPP_HIDE_FROM_ABI to_chars_result
 to_chars(char* __first, char* __last, _Tp __value)
 {
   using _Type = __make_32_64_or_128_bit_t<_Tp>;
-  static_assert(!is_same<_Type, void>::value, "unsupported integral type used in to_chars");
+  static_assert(!is_void<_Type>::value, "unsupported integral type used in to_chars");
   return std::__to_chars_itoa(__first, __last, static_cast<_Type>(__value), is_signed<_Tp>());
 }
 
 template <typename _Tp, typename enable_if<is_integral<_Tp>::value, int>::type = 0>
 inline _LIBCPP_HIDE_FROM_ABI to_chars_result
 to_chars(char* __first, char* __last, _Tp __value, int __base)
 {
   _LIBCPP_ASSERT(2 <= __base && __base <= 36, "base not in [2, 36]");

libcxx/test/std/containers/container.adaptors/queue/queue.defn/emplace.pass.cpp

void test_return_type() {
     typedef typename Queue::container_type Container;
     typedef typename Container::value_type value_type;
     typedef decltype(std::declval<Queue>().emplace(std::declval<value_type &>())) queue_return_type;
 
 #if TEST_STD_VER > 14
     typedef decltype(std::declval<Container>().emplace_back(std::declval<value_type>())) container_return_type;
     static_assert(std::is_same<queue_return_type, container_return_type>::value, "");
 #else
-    static_assert(std::is_same<queue_return_type, void>::value, "");
+    static_assert(std::is_void<queue_return_type>::value, "");
 #endif
 }
 
 int main(int, char**)
 {
     test_return_type<std::queue<int> > ();
     test_return_type<std::queue<int, std::list<int> > > ();
 

libcxx/test/std/containers/container.adaptors/stack/stack.defn/emplace.pass.cpp

void test_return_type() {
     typedef typename Stack::container_type Container;
     typedef typename Container::value_type value_type;
     typedef decltype(std::declval<Stack>().emplace(std::declval<value_type &>()))     stack_return_type;
 
 #if TEST_STD_VER > 14
     typedef decltype(std::declval<Container>().emplace_back(std::declval<value_type>())) container_return_type;
     static_assert(std::is_same<stack_return_type, container_return_type>::value, "");
 #else
-    static_assert(std::is_same<stack_return_type, void>::value, "");
+    static_assert(std::is_void<stack_return_type>::value, "");
 #endif
 }
 
 int main(int, char**)
 {
     test_return_type<std::stack<int> > ();
     test_return_type<std::stack<int, std::vector<int> > > ();