called copy constructor in foreach with ref on Range

[Jonathan M Davis]

On Monday, June 22, 2020 3:33:08 PM MDT H. S. Teoh via Digitalmars-d-learn 
wrote:
> On Mon, Jun 22, 2020 at 09:11:07PM +0000, Stanislav Blinov via Digitalmars-
d-learn wrote:
> > That is not true. Copying of forward ranges is absolutely fine. It's
> > what the current `save()` primitive is supposed to do. It's the
> > copying of input ranges should just be rejected, statically.
>
> Jonathan is coming from the POV of generic code.  The problem with move
> leaving the original range in its .init state isn't so much that it will
> crash or anything (even though as you said that does indicate a flaw in
> the range's implementation), but that the semantics of generic code
> changes in subtle ways. For example:
>
>   auto myGenericFunc(R)(R r) {
>       ...
>       foreach (x; r) {
>           doSomething(x);
>       }
>       if (!r.empty)
>           doSomethingElse(r);
>       ...
>   }
>
> Suppose for argument's sake that the above foreach/if structure is an
> essential part of whatever algorithm myGenericFunc is implementing. Now
> there's a problem, because if R has array-like semantics, then the
> algorithm will do one thing, but if R has reference-like or move
> semantics, then the behaviour of the algorithm will be different, even
> if both ranges represent the same sequence of input values.
>
> Note that in real-life code, this problem can be far more subtle than a
> blatant foreach loop and if statement like the above. For example,
> consider a function that drops the first n elements of a range. Your
> generic function might want to pop the first n elements then do
> something else with the rest of the range.  Well, if you write it the
> obvious way:
>
>   auto myAlgo(R)(R r) {
>       size_t n = ...;
>       dropFirstN(r, n);
>       ... // do something else with r
>   }
>
> then you have a subtle bug, because the state of r after the call to
> dropFirstN might be completely different depending on whether r behaves
> like an array or like a by-reference or move type.

Exactly. It's because of issues like this that generic, range-based
functions need to be tested with a variety of range types - including
reference types. Without that, bugs are routinely missed. I think that a
fairly large percentage of Phobos (maybe even all of it) gets that right
now, because we've added tests for reference type ranges, but there used to
be quite a few bugs in Phobos, because there were no such tests. It's
frequently the case that people write range-based code under the assumption
that ranges will act like dynamic arrays, and their code then also works
with many ranges which have similar copying behavior, but it doesn't work
with all range types. Such problems don't generally get caught without
extensive testing. It matters a lot less for code within a program that
doesn't use a large variety of range types, but for libraries using generic
functions, it's a must.

If we do actually rework the range API as has occasionally been discussed,
it would be _really_ nice if we could more thoroughly restrict the copying
semantics of ranges so that some of these problems go away, but without such
a redesign, we're stuck with such problems. And even with a redesign, the
best fix for it is far from obvious, because basic input ranges and forward
ranges inherently have different copying semantics. We can probably require
that copying forward ranges always results in an independent copy (thereby
essentially having value semantics), but basic input ranges can't really be
value types, because if they could, they could be forward ranges, meaning
that they're generally going to either be reference types or
pseudo-reference types, which of course have different copying semantics.
So, as long as the range API is set up so that the same code can operate on
both basic input and forward ranges, we pretty much inherently have a
problem with the copying semantics in ranges being inconsistent - though
requiring value semantics for forward ranges would still be a significant
improvement.

- Jonathan M Davis