-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor <limits>
usage
#4634
Refactor <limits>
usage
#4634
Conversation
Based on microsoft#4627 fixup that extracts `_In_range` This also makes some place dirctly using `_In_range`, but mostly `_Max_limit` is used
libc++ tests failures are reported upstream as LLVM-90345 |
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: A. Jiang <[email protected]>
This reverts commit dd292a8.
I'm not sure if this change is correct though, as maybe the parts I've changed need to support custom integer-like types:
These custom integer-like types may provide a specialization of |
Integer-like types are not supposed to be customizable. They're either integer types or integer-class types provided by the implementation. The requirements in [iterator.concept.winc] don't seem implementable if a user-defined type is allowed to be an integer-class type. In MSVC STL, we only need to specially handle
Nope. The standard clearly says that their |
Noticed while attempting microsoft/STL#4634
and actually improving throughput
* This was unnecessarily asserting *strictly* less than `ULONG_MAX`, not that it mattered. * We can/should use Standard `in_range` because `<semaphore>` is C++20. * We control `_Ten_days`, so this should be an `_STL_INTERNAL_STATIC_ASSERT`. * As a scalar, `_Ten_days` doesn't need to be `static constexpr`, it can/should be plain `constexpr`.
We know `is_integral_v<_Elem>` and `is_signed_v<_Elem>` so this is definitely ok.
This is `_Integer_like _Int`, which is ok because we handle `_Signed128` and `_Unsigned128`. The first case is simple: `_Int` is known to be unsigned, and we want the maximum. The second case requires a bit of thought. `_Int` is known to be signed, and `_UInt` is its unsigned counterpart. This code wants the maximum `_Int`, stored in `_UInt` type. The new function makes this clearer than the old math.
The following headers are no longer including `<limits>`: * C++14 `<xmemory>` * C++20 `<format>` * C++23 `<mdspan>` The installed base of C++20/23 code is far smaller, and far more modern. We don't need escape hatches for them. However, we should have an escape hatch for the `<xmemory>` change, as I expect that this will otherwise be too disruptive.
Thanks! I pushed several commits:
|
I thought that was #4646 |
Sorry, I've been context-switched too much for the last couple of weeks and I lost track of all of the in-flight changes. Let's consolidate into this one. I'll validate and push changes. |
Co-authored-by: Alex Guteniev <[email protected]>
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
…de `<cwchar>` in `<limits>`". Co-authored-by: A. Jiang <[email protected]>
Removing Because it's closely related, I partially resurrected @frederick-vs-ja's #3631, taking the changes to make (There were several breaks assuming each step of the Lots Of Stuff => |
<limits>
: avoid including to improve throughput<limits>
usage
Thanks for cleaning this up! The |
Based on #4627 fixup that extracts
_In_range
.This also makes some place directly using
_In_range
, but mostly_Max_limit
is used.