ericniebler / range-v3

Range library for C++14/17/20, basis for C++20's std::ranges
Other
4.05k stars 437 forks source link

Fail with std::back_inserter #1645

Open sergegers opened 2 years ago

sergegers commented 2 years ago

After upgrading Visual Studio to 2022 Preview 3 the following code is failed


#include <vector>
#include <range/v3/algorithm/copy.hpp>

int main()
{
    std::vector<int> from_vector(10);
    std::vector<int> to_vector;

    ranges::copy(from_vector, std::back_inserter(to_vector));
}

As I can see the compilation is failed because std::back_insert_iterator<std::vector> doesn't satisfy concept weakly_incrementable -> semiregular -> default_constructible. But back_insert_iterator is not default constructible! How did it work before? Anyway it looks like a bug. The code with STL range implementation works fine


#include <vector>
#include <algorithm>
#include <iterator>

int main()
{
    std::vector<int> from_vector(10);
    std::vector<int> to_vector;

    std::ranges::copy(from_vector, std::back_inserter(to_vector));
}

because weakly_incrementable implementation is look like

template <class _Ty>
concept weakly_incrementable = movable<_Ty> && requires(_Ty __i) {
    typename iter_difference_t<_Ty>;
    requires _Signed_integer_like<iter_difference_t<_Ty>>;
    { ++__i } -> same_as<_Ty&>;
    __i++;
};

and doesn't require default construction.

sergegers commented 2 years ago

@ericniebler pls, leave a comment. Did I make a mistake in my conclusions?

ericniebler commented 2 years ago

You're correct. The solution is to use the back_inserter from range-v3, and to wait for Microsoft to update its back_inserter implementation for C++20.

JohelEGP commented 2 years ago

Range-v3 hasn't adopted https://github.com/cplusplus/draft/commit/479c6a8f333fa8ea6fac11563d715f0cba376d82#diff-0043c904d3e034a77b44feeddf18719e63e646d55db1c564f2a5defe7330c9eaL1321 nor its previous relaxation, as seen at https://github.com/ericniebler/range-v3/blob/master/include/range/v3/iterator/concepts.hpp#L257. My guess is that some wrongly hard-coded logic made Range-v3 fall back to using its own concepts due to not detecting that your implementation still provides them.

sergegers commented 2 years ago

So you are waiting when P2325R3 leaks in production compiler. Thank you @ericniebler , @JohelEGP .

huixie90 commented 2 years ago

You're correct. The solution is to use the back_inserter from range-v3, and to wait for Microsoft to update its back_inserter implementation for C++20.

what is the current status of weakly_incrementable ? c++20 version seems to require default_initializable, but the current working draft removes the requirement

JohelEGP commented 2 years ago

From https://github.com/cplusplus/papers/issues/1007

with the recommendation that implementations retroactively apply it to C++20.

huixie90 commented 2 years ago

From cplusplus/papers#1007

with the recommendation that implementations retroactively apply it to C++20.

what does this "it" refer to? apply "P2325R2" or apply "default_constructible" ? apply "P2325R2" means that the implementation should remove the need for default_constructible for C++20 as well?

JohelEGP commented 2 years ago

Yes, the paper itself.

ericniebler commented 2 years ago

Range-v3 hasn't kept up with the changes made in std::ranges. This is mostly because I lack time. Patches welcome.