Skip to content
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

Modernize ranges::swap_ranges and ranges::distance #1062

Merged
merged 9 commits into from
Aug 1, 2020

Conversation

miscco
Copy link
Contributor

@miscco miscco commented Jul 20, 2020

This swaps the implementation of the the last two algorithms to bridge the distance to the other ones.

@miscco miscco requested a review from a team as a code owner July 20, 2020 14:57
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Jul 20, 2020
@CaseyCarter
Copy link
Member

This swaps the implementation of the the last two algorithms to bridge the distance to the other ones.

I think I'm having a negative influence on you ;)

stl/inc/xutility Outdated Show resolved Hide resolved
@miscco
Copy link
Contributor Author

miscco commented Jul 21, 2020

BTW can somebody tell my why this breaks, because it totally works on my machine (Aka why does it not swap the ranges?)

@statementreply
Copy link
Contributor

statementreply commented Jul 21, 2020

why does it not swap the ranges?

There is a bug in ranges::iter_swap (#1067).

@miscco
Copy link
Contributor Author

miscco commented Jul 21, 2020

why does it not swap the ranges?

There is a bug in ranges::iter_swap (#1067).

Hooray for increased test coverage

@CaseyCarter
Copy link
Member

CaseyCarter commented Jul 21, 2020

This is blocked on #1072. @miscco, could you verify that #1072 fixes this?

@CaseyCarter CaseyCarter self-requested a review July 21, 2020 23:32
@CaseyCarter CaseyCarter added the blocked Something is preventing work on this label Jul 21, 2020
@miscco
Copy link
Contributor Author

miscco commented Jul 22, 2020

At least on my machine it works now, but it did before so ...

@StephanTLavavej StephanTLavavej added the ranges C++20/23 ranges label Jul 22, 2020
@statementreply
Copy link
Contributor

stl-lit.py: C:\Users\He\source\repos\STL\tests\utils\stl\test\format.py:203: note: Build step failed unexpectedly.
Command: "C:\Program Files\LLVM\bin\clang-cl.EXE" "/FeC:\Users\He\source\repos\STL\out\build\x64\tests\std\tests\P0896R4_ranges_alg_swap_ranges\15\P0896R4_ranges_alg_swap_ranges.exe" "/FoC:\Users\He\source\repos\STL\out\build\x64\tests\std\tests\P0896R4_ranges_alg_swap_ranges\15\P0896R4_ranges_alg_swap_ranges.obj" "/IC:\Users\He\source\repos\STL\out\build\x64\out\inc" "/IC:/Users/He/source/repos/STL/llvm-project/libcxx/test/support" "/IC:/Users/He/source/repos/STL/tests/std/include" "-m64" "/nologo" "/Od" "/W4" "/w14061" "/w14242" "/w14265" "/w14582" "/w14583" "/w14587" "/w14588" "/w14749" "/w14841" "/w14842" "/w15038" "/w15214" "/w15215" "/w15216" "/w15217" "/sdl" "/WX" "/Zc:strictStrings" "/D_ENABLE_STL_INTERNAL_CHECK" "/bigobj" "/FIforce_include.hpp" "/w14365" "/D_ENFORCE_FACET_SPECIALIZATIONS=1" "/w14640" "/Zc:threadSafeInit-" "/EHsc" "/std:c++latest" "-fno-ms-compatibility" "-fno-delayed-template-parsing" "/MD" "C:\Users\He\source\repos\STL\tests\std\tests\P0896R4_ranges_alg_swap_ranges\test.cpp" "/link" "/LIBPATH:C:\Users\He\source\repos\STL\out\build\x64\out\lib\amd64" "/LIBPATH:C:\Program Files (x86)\Microsoft Visual Studio\2019\Preview\VC\Tools\MSVC\14.27.29109\lib\x64" "/MANIFEST:EMBED"
Exit Code: 1
Standard Error:
--
C:\Users\He\source\repos\STL\tests\std\tests\P0896R4_ranges_alg_swap_ranges\test.cpp(67,19): error: static_assert expression is not an integral constant expression
    STATIC_ASSERT((test_in_in<instantiator, int, int>(), true));
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:/Users/He/source/repos/STL/tests/std/include\range_algorithm_support.hpp(15,42): note: expanded from macro 'STATIC_ASSERT'
#define STATIC_ASSERT(...) static_assert(__VA_ARGS__, #__VA_ARGS__)
                                         ^~~~~~~~~~~
C:\Users\He\source\repos\STL\out\build\x64\out\inc\xutility(682,111): note: constexpr evaluation hit maximum step limit; possible infinite loop?
            _NODISCARD constexpr decltype(auto) operator()(_Ty&& _Val) const noexcept(_Choice<_Ty>._No_throw) {
                                                                                                              ^
C:\Users\He\source\repos\STL\out\build\x64\out\inc\xutility(1033,57): note: in call to '&iter_move->operator()(_First2)'
            iter_value_t<remove_reference_t<_Xty>> _Tmp(_RANGES iter_move(_XVal));
                                                        ^
C:\Users\He\source\repos\STL\out\build\x64\out\inc\yvals_core.h(1218,20): note: expanded from macro '_RANGES'
#define _RANGES    ::std::ranges::
                   ^
C:\Users\He\source\repos\STL\out\build\x64\out\inc\xutility(1071,25): note: in call to '_Iter_exchange_move(_First2, _First1)'
                        _Iter_exchange_move(static_cast<_Ty2&&>(_Val2), static_cast<_Ty1&&>(_Val1));
                        ^
C:\Users\He\source\repos\STL\out\build\x64\out\inc\algorithm(3273,17): note: in call to '&iter_swap->operator()(_First1, _First2)'
                _RANGES iter_swap(_First1, _First2);
                ^
C:\Users\He\source\repos\STL\out\build\x64\out\inc\yvals_core.h(1218,20): note: expanded from macro '_RANGES'
#define _RANGES    ::std::ranges::
                   ^
C:\Users\He\source\repos\STL\out\build\x64\out\inc\algorithm(3254,29): note: in call to '_Swap_ranges_unchecked({&input1[0]}, {&input1[3]}, {&input2[0]}, {&input2[3]})'
            auto _UResult = _Swap_ranges_unchecked(
                            ^
C:\Users\He\source\repos\STL\tests\std\tests\P0896R4_ranges_alg_swap_ranges\test.cpp(54,27): note: in call to '&swap_ranges->operator()(wrapped_input1, wrapped_input2)'
            auto result = swap_ranges(wrapped_input1, wrapped_input2);
                          ^
C:/Users/He/source/repos/STL/tests/std/include\range_algorithm_support.hpp(877,9): note: in call to 'call()'
        Continuation::template call<Args...,
        ^
C:/Users/He/source/repos/STL/tests/std/include\range_algorithm_support.hpp(904,9): note: in call to 'call()'
        Continuation::template call<Args...,
        ^
C:/Users/He/source/repos/STL/tests/std/include\range_algorithm_support.hpp(1038,5): note: in call to 'call()'
    with_input_ranges<with_input_ranges<Instantiator, Element2>, Element1>::call();
    ^
C:\Users\He\source\repos\STL\tests\std\tests\P0896R4_ranges_alg_swap_ranges\test.cpp(67,20): note: in call to 'test_in_in()'
    STATIC_ASSERT((test_in_in<instantiator, int, int>(), true));
                   ^
1 error generated.

clang ran out of constexpr evaluation limit on my machine.

@CaseyCarter CaseyCarter removed the blocked Something is preventing work on this label Jul 30, 2020
@CaseyCarter
Copy link
Member

This is no longer blocked on #1062: we should be able to merge master and get this cleaned up and across the finish line.

Copy link
Member

@StephanTLavavej StephanTLavavej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - I will push a change to fix a comment (I verified against WG21-N4861 that the test code was correct) and to complete the using namespace std; change. Thanks!

tests/std/tests/P0896R4_ranges_alg_swap_ranges/test.cpp Outdated Show resolved Hide resolved
tests/std/tests/P0896R4_ranges_alg_swap_ranges/test.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this Jul 31, 2020
@StephanTLavavej StephanTLavavej merged commit accaf18 into microsoft:master Aug 1, 2020
@StephanTLavavej
Copy link
Member

Thanks for the ongoing modernizations! 🚀

@miscco miscco deleted the ranges_util branch August 2, 2020 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something can be improved ranges C++20/23 ranges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants