Unify function overloads in std.range

[wilzbach]

Created a new PR due to CI errors in #5605

[dlang-bot]

Thanks for your pull request, @wilzbach!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

[wilzbach]

Ping me if it happens again, I'll see which SHA is wrong.

Ping :P

[andralex]

This is new behavior - now chooseAmong accepts wrong arguments and errs in the implementation. That is acceptable, but the current prevalent approach is to eliminate unacceptable arguments in the constraint. To do so:

auto chooseAmong(Ranges...)(size_t index, Ranges rs)
if (Ranges.length >= 2
    && allSatisfy!(isInputRange, staticMap!(Unqual, Ranges))
    && !is(CommonType!(staticMap!(ElementType, Ranges)) == void))
{
    ...
}

It's also nicely readable - there have got to be two or more arguments, all of which must be input ranges, which must have some common element type.

[wilzbach]

Hehe this paradigm switch has happened a couple of months ago, see e.g. @quickfur's excellent explananation at #5148 (comment):

And just for the record, the last bit is what I meant in various forum posts that user-facing overloads should have sig constraints that reflect intent rather than implementation: the intent of each is that it would accept anything that either supports foreach or has a range API. So those should be what constitute the sig constraints of each.
There may be, however, some ranges that satisfy these constraints but which aren't handled by the current implementation; these should be caught by the final else clause in the static if inside each overload body, with a helpful error message explaining to the user why it didn't work ("support for range with characteristics XYZ isn't implemented yet", or "cannot call each with range with characteristics XYZ because PQR").

In this case one could argue for both sides, but imho having a concrete message with what went wrong helps the user a lot more than "X didn't match constraints".
I don't feel strongely about this particular case though, so I'm happy to change it as suggested if that's the only thing that blocks this PR. Other Opinions?

[PetarKirov]

I agree 100% with @quickfur's comment, but in this case requiring that there's a common element type between all ranges is part of the intent, not just an implementation detail.
Unless we decide to return a range of Algebraic!(E1, E2, ...) elements, I don't think this will change in the future.

[quickfur]

I agree. Given the current implementation, since the return type must be bound at compile-time but the decision of which range to return is made at runtime, we must require returning a common element type.

Please add this to the sig constraint, and we're good to go.

[andralex]

Apparently not trivial :)

[andralex]

ping @wilzbach

[wilzbach]

ping @wilzbach

I will get to this soon, just busy with my day job as a student ;-)
However, there's enough enough stuff in the queue that would require your attention.
An easy one is:

#5575

And the full list:

https://github.com/dlang/phobos/pulls?q=is%3Aopen+is%3Apr+label%3A%40andralex

Finally got around adding the constraint.

[wilzbach]

ping @wilzbach

Alrighty, should be finally good to go. Sorry for the hold-up.

[andralex]

@wilzbach nice comeback :)

[andralex]

Wunderbar