Extremely cryptic error message - bug?

[rnikander]

This error message has two problems:

1. It doesn't mention my offending line of code.
2. It's so cryptic that I can't decipher what it's complaining about.

I get that template C++ error messages are difficult but this seems like it might be far enough to be considered a bug.

Perhaps it's specific to the compiler. I'm using Clang 15.0.1

#include <iostream>
#include <vector>
#include <range/v3/all.hpp>

int main(int argc, char *argv[])
{
    namespace rv = ranges::views;
    std::vector<int> points = {10,11,12,13};
    // This is what I meant:
    // auto point_idxs = rv::concat( rv::iota(0, (int)points.size()), rv::single(0) );
    // But I wrote this:
    auto point_idxs = rv::concat( rv::iota(points.size()), rv::single(0) );    
    for (auto chunk : point_idxs | rv::sliding(2)) {
        std::cout << chunk << "\n";
    }
    return 0;
}

The error message is attached.

error.txt

[JohelEGP]

detail::diffmax_t's conversion operator is explicit, and the implementation of concat fails to account for that.

[brevzin]

detail::diffmax_t's conversion operator is explicit, and the implementation of concat fails to account for that.

Why is it explicit? It's easy enough to go through and change https://github.com/ericniebler/range-v3/blob/689b4f3da769fb21dd7acf62550a038242d832e5/include/range/v3/view/concat.hpp#L176 to be

ranges::advance(it.get(), static_cast<iter_difference_t<I>>(n));

in the dozen or so odd places where this needs to happen, but ...?

[brevzin]

That's the rule we ended up with in the standard, courtesy of P1522 but I'm not ... sure why.

[CaseyCarter]

Because potentially-narrowing conversions in C++ should not happen implicitly.

[JohelEGP]

Which was also changed by https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2393r1.html to be conditionally explicit.

[JohelEGP]

The proposed views::concat https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2542r2.html seems to use a different strategy to advance, even though both this implementation and the proposed one are non-borrowed.

[CaseyCarter]

The proposed views::concat https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2542r2.html seems to use a different strategy to advance, even though both this implementation and the proposed one are non-borrowed.

@huixie90 Every use of + and - with an iterator and an integral type in that wording needs to convert the integral to the iterator's difference type. We don't require random access iterators to accept arbitrary integer-like types, they're only required to accept their difference type. (Queue war story about x + 1 in the implementation of vector breaking a user program.)

[JohelEGP]

Looking at range-v3, the integer type passed to advance is the common type of the difference types of the ranges in concat_view. The conversion to the actual ith iterator type's difference type should be in range.

[brevzin]

Because potentially-narrowing conversions in C++ should not happen implicitly.

So should we use narrow_cast on all such conversions? https://github.com/ericniebler/range-v3/blob/689b4f3da769fb21dd7acf62550a038242d832e5/include/range/v3/view/span.hpp#L51-L60

[huixie90]

The proposed views::concat https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2542r2.html seems to use a different strategy to advance, even though both this implementation and the proposed one are non-borrowed.

@huixie90 Every use of + and - with an iterator and an integral type in that wording needs to convert the integral to the iterator's difference type. We don't require random access iterators to accept arbitrary integer-like types, they're only required to accept their difference type. (Queue war story about x + 1 in the implementation of vector breaking a user program.)

thanks for spotting that. If such conversion is explicit, there must be a reason (e.g narrowing). Instead of making concat to do the conversion (potentially narrowing) , would it be more appropriate to disallow such inputs all together by adding more constraints to concat?

[JohelEGP]

Irrespective of the range of values in the the difference type, is the size of the range. The implementation is calling advance(it, n), so n must already be such that conceptually [it, it + n) is a valid range. It just so happens that it's using a bad type for n. Otherwise, it's a bug in the implementation.

[CaseyCarter]

So should we use narrow_cast on all such conversions?

I think this would be a good idea if narrow_cast were extended to accept integer-like types.

If such conversion is explicit, there must be a reason (e.g narrowing). Instead of making concat to do the conversion (potentially narrowing) , would it be more appropriate to disallow such inputs all together by adding more constraints to concat?

That would effectively mean that concat only works when all constituent ranges have the same difference type, which is IMO throwing out the baby with the bathwater. Like Johel says, we always know that the actual values used are in the representable range, otherwise e.g. get<N>(it_) += steps; would have UB regardless of the type of steps.