[libc++] Consistently unparenthesize `numeric_limits<T>::max`. NFCI.

[• Quuxplusone]

Summary

I think people were sometimes parenthesizing (foo::max)() out of misplaced concern that an unparenthesized foo::max() would trip up Windows' max(a,b) macro. However, this is not the case: max(a,b) should be tripped up only by an unparenthesized call to foo::max(a,b), and in fact we already do _VSTD::max(a,b) all over the place anyway without any guards.

However, in order to do it without guards, we must also wrap the header in _LIBCPP_PUSH_MACROS, which <span> was not.

(For reference: The charconv pieces were originally added in D41458.)

[ldionne]

Don't wait for CI, this is fine. Thanks!

[• Quuxplusone]

Actually, this does break libcxx/test/libcxx/min_max_macros.compile.pass.cpp — but now I'm trying to figure out how the existing use of min() and max(), e.g. in <istream>, doesn't break that test!

EDIT: oh, that's easy: that test skips <istream> and <latch> and so on. But <chrono> and <limits> and <random> are all there... how do those headers not break it...?

[• Quuxplusone]

Ah, I see it now.

$ git grep -L _LIBCPP_PUSH_MACROS ../libcxx/include/ | xargs git grep -l '::max('
../libcxx/include/barrier
../libcxx/include/latch
../libcxx/include/semaphore
../libcxx/include/span

I wouldn't mind making the patch for barrier/latch/semaphore (basically D68480 should have added them to lists-of-all-the-headers in a couple more places than it did, and min_max_macros.compile.pass.cpp was one of those places) — but I suspect you might want to do that yourself, so I won't do it unless you ask me to.

[curdeius]

In D92240#2420684, @Quuxplusone wrote:

I wouldn't mind making the patch for barrier/latch/semaphore (basically D68480 should have added them to lists-of-all-the-headers in a couple more places than it did, and min_max_macros.compile.pass.cpp was one of those places)

@ldionne, should that be added to NOTES.txt/(future)contributing.rst?
@Quuxplusone, are there any other places you found?

[• Quuxplusone]

In D92240#2421040, @curdeius wrote:

In D92240#2420684, @Quuxplusone wrote:

I wouldn't mind making the patch for barrier/latch/semaphore (basically D68480 should have added them to lists-of-all-the-headers in a couple more places than it did, and min_max_macros.compile.pass.cpp was one of those places)

@ldionne, should that be added to NOTES.txt/(future)contributing.rst?
@Quuxplusone, are there any other places you found?

Technically no, I hadn't looked. But now that I've looked, yes, of the places I know as "big lists of headers", the new synchronization-primitive headers are missing from exactly two of them:
include/CMakeLists.txt (OK)
include/module.modulemap (OK)
test/libcxx/double_include.sh.cpp (OK)
test/libcxx/min_max_macros.compile.pass.cpp (missing)
test/libcxx/no_assert_include.compile.pass.cpp (missing)

[curdeius]

Thank you!

[ldionne]

In D92240#2420684, @Quuxplusone wrote:

...

[...] — but I suspect you might want to do that yourself, so I won't do it unless you ask me to.

Feel free to go ahead! Thank you both @Quuxplusone and @curdeius for straightening this out. Libc++ doesn't get a lot of this kind of love, and it needs it!

libcxx/include/charconv

 };
 
 template <typename _Tp>
 inline _LIBCPP_INLINE_VISIBILITY bool
 __mul_overflowed(unsigned char __a, _Tp __b, unsigned char& __r)
 {
     auto __c = __a * __b;
     __r = __c;
-    return __c > (numeric_limits<unsigned char>::max)();
+    return __c > numeric_limits<unsigned char>::max();
 }
 
 template <typename _Tp>
 inline _LIBCPP_INLINE_VISIBILITY bool
 __mul_overflowed(unsigned short __a, _Tp __b, unsigned short& __r)
 {
     auto __c = __a * __b;
     __r = __c;
-    return __c > (numeric_limits<unsigned short>::max)();
+    return __c > numeric_limits<unsigned short>::max();
 }
 
 template <typename _Tp>
 inline _LIBCPP_INLINE_VISIBILITY bool
 __mul_overflowed(_Tp __a, _Tp __b, _Tp& __r)
 {
     static_assert(is_unsigned<_Tp>::value, "");
 #if !defined(_LIBCPP_COMPILER_MSVC)
     return __builtin_mul_overflow(__a, __b, &__r);
 #else
-    bool __did = __b && ((numeric_limits<_Tp>::max)() / __b) < __a;
+    bool __did = __b && (numeric_limits<_Tp>::max() / __b) < __a;
     __r = __a * __b;
     return __did;
 #endif
 }
 
 template <typename _Tp, typename _Up>
 inline _LIBCPP_INLINE_VISIBILITY bool
 __mul_overflowed(_Tp __a, _Up __b, _Tp& __r)
if (__neg)
         {
             __x = __complement(__x);
             memcpy(&__value, &__x, sizeof(__x));
             return __r;
         }
     }
     else
     {
-        if (__x <= (__tl::max)())
+        if (__x <= __tl::max())
         {
             __value = __x;
             return __r;
         }
     }
 
     return {__r.ptr, errc::result_out_of_range};
 }
__from_chars_atoi(const char* __first, const char* __last, _Tp& __value)
     return __subject_seq_combinator(
         __first, __last, __value,
         [](const char* __first, const char* __last,
            _Tp& __value) -> from_chars_result {
             __output_type __a, __b;
             auto __p = __tx::__read(__first, __last, __a, __b);
             if (__p == __last || !__in_pattern(*__p))
             {
-                __output_type __m = (numeric_limits<_Tp>::max)();
+                __output_type __m = numeric_limits<_Tp>::max();
                 if (__m >= __a && __m - __a >= __b)
                 {
                     __value = __a + __b;
                     return {__p, {}};
                 }
             }
             return {__p, errc::result_out_of_range};
         });
return __subject_seq_combinator(
                     }
                 }
                 else
                     break;
             }
 
             if (__p == __last || !__in_pattern(*__p, __base))
             {
-                if ((__tl::max)() - __a >= __b)
+                if (__tl::max() - __a >= __b)
                 {
                     __value = __a + __b;
                     return {__p, {}};
                 }
             }
             return {__p, errc::result_out_of_range};
         },
         __base);

libcxx/include/span

 #include <cstddef>      // for byte
 #include <iterator>     // for iterators
 #include <type_traits>  // for remove_cv, etc
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
+_LIBCPP_PUSH_MACROS
+#include <__undef_macros>
+
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #if _LIBCPP_STD_VER > 17
 
-inline constexpr size_t dynamic_extent = (numeric_limits<size_t>::max)();
+inline constexpr size_t dynamic_extent = numeric_limits<size_t>::max();
 template <typename _Tp, size_t _Extent = dynamic_extent> class span;
 
 
 template <class _Tp>
 struct __is_span_impl : public false_type {};
 
 template <class _Tp, size_t _Extent>
 struct __is_span_impl<span<_Tp, _Extent>> : public true_type {};

 
 template<class _Container>
     span(const _Container&) -> span<const typename _Container::value_type>;
 
 #endif // _LIBCPP_STD_VER > 17
 
 _LIBCPP_END_NAMESPACE_STD
 
+_LIBCPP_POP_MACROS
+
 #endif // _LIBCPP_SPAN