Miscellaneous Cleanups#3178
Merged
StephanTLavavej merged 10 commits intomicrosoft:mainfrom Oct 26, 2022
Merged
Conversation
Make `transform_view::iterator::_Verify_offset` always `noexcept` by avoiding checks when the underlying iterator doesn't support `_Verify_offset`. Make that function and `elements_view::iterator::_Verify_offset` available only when `_IDL != 0` **fixing a bug in `elements_view::iterator`**. Remove the now unused `_NOEXCEPT_IDL0` machinery. Fixes microsoft#1269
... to `if (cond) y(); else x();` Drive-by: Let's `static_assert` only when running the test suite.
* Simplify nested requirement in `HasPeek` * Remove excess empty line
... from `instantiator::call`. Reduces compile + run time by 27%.
strega-nil-ms
approved these changes
Oct 25, 2022
Contributor
strega-nil-ms
left a comment
There was a problem hiding this comment.
LGTM! (obviously, fix the red cross)
StephanTLavavej
requested changes
Oct 25, 2022
StephanTLavavej
approved these changes
Oct 25, 2022
Member
|
I've pushed a commit to fix the test failures because Additionally, I split it into two static assertions, partially to make things line up nicer, but also to follow our usual guidance to assert X and Y separately instead of |
transform_view iterators are validated only when the underlying iterators are.
StephanTLavavej
approved these changes
Oct 25, 2022
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
CaseyCarter
commented
Oct 25, 2022
StephanTLavavej
approved these changes
Oct 25, 2022
Member
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Member
|
Thanks for finding and fixing these issues! 🏚️ 🧹 🏡 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mostly things I noticed while reviewing the last import batch at the last minute that were not worth resetting testing for, but does include a couple of "old" changes as well.
Separate commits to ease review:
basic_string::_Move_construct_from_substralready knows the allocator": avoids passing an allocator to this member function which can simply retrieve the stored allocator._Verify_offset(-n)against-INT_MIN" (in<ranges>): we had a couple of places that guarded against overflow negating the argument to_Verify_offset, we should do so consistently._NOEXCEPT_IDL0": Removes this variation innoexceptbetween debug and release builds - it was a terrible idea. Maketransform_view::iterator::_Verify_offsetalwaysnoexceptby avoiding checks when the underlying iterator doesn't support_Verify_offset. Make that function andelements_view::iterator::_Verify_offsetavailable only when_IDL != 0fixing a bug inelements_view::iteratorthat disabled the check. Remove the now unused_NOEXCEPT_IDL0machinery. Fixes <ranges>: Questionable conditional noexcept #1269.<execution>": Fixes <execution>: Consider separating type and data member declarations #330.<system_error>: Correctif (!cond) x(); else y()": ... toif (cond) y(); else x();. Drive-by: Let'sstatic_assertonly when running the test suite.tests/P2278R4_basic_const_iteratornitpicks":HasPeektests/P2322R6_ranges_alg_fold: factor out non-dependent tests": ... from the dependent test cases ininstantiator::call. Reduces compile + run time by 27%.