called copy constructor in foreach with ref on Range

[Jonathan M Davis]

On Monday, June 22, 2020 3:11:07 PM MDT Stanislav Blinov via Digitalmars-d-
learn wrote:
> On Monday, 22 June 2020 at 20:51:37 UTC, Jonathan M Davis wrote:
> > You're unlikely to find much range-based code that does that
> > and there really isn't much point in doing that. Again, copying
> > isn't the problem. It's using the original after making the
> > copy that's the problem.
>
> Copy *is* the problem. If you can't make a copy (i.e. you get a
> compilation error) - you don't have a problem, the compiler just
> found a bug for you. Sadly, historically D's libraries were built
> around lots of redundant copying, even though there are very few
> cases where copies are actually required. The reason why we're
> "unlikely to find much range-based code that does that [move]" is
> (a) sloppy or shortsighted design and (b) poor language support.
> The latter hopefully stands to change.

As things stand, uncopyable ranges aren't really a thing, and common range
idiomns rely on ranges being copyable. We'd need some major redesigning to
make uncopyable ranges work, and personally, I don't think that it's worth
the trouble.

> > And moving doesn't fix anything, since the original variable is
> > still there
> > (just in its init state, which would still be invalid to use in
> > generic code and could outright crash in some cases if you
> > tried to use it - e.g. if it were a class reference, since it
> > would then be null).
>
> Eh? A range in 'init' state should be an empty range. If you get
> a crash from that then there's a bug in that range's
> implementation, not in user code.

The range API has no requirement that the init value of a range be empty,
and any generic code which relies on such behavior is buggy. In the case of
classes, it would outright crash, because the init value would be null. I
agree that ideally the range API would require that the init state of a
range be a valid, empty range, but that's simply not how it works right now.
In order to make it work that way, we'd have to redesign the range API in at
least some respects (e.g. getting rid of save and making it illegal for
classes to be forward ranges).

> > So, code that does a move could accidentally use the original
> > range after the move and have bugs just like code that copies
> > the range has bugs if the original is used after the copy has
> > been made. So, the rule of thumb is not that you should avoid
> > copying ranges. It's that once you've copied a range, you
> > should then use only the copy and not the original.
>
> 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.

As things stand, it is _not_ true that it's safe to copy forward ranges and
then use the original. Sure, it will work for some ranges, but for others it
won't. The entire reason that save exists is for ranges that are classes,
because copying them does not result in an independent range. The range API
does not require that copying a range result in an independent copy. It's
not even guaranteed that copying a range that's a struct will result in an
indpendent copy. Depending on the range type, copying it could result in an
independent copy, or it could result in a reference to to the original
range, or it could even result in part of the state being copied and part of
it being a reference (e.g. if the current front is stored directly in the
range, but the iteration state is stored by reference). If you rely on
copying a range resulting in an independent copy, you will have buggy code -
even if it's a forward range. The _only_ way that the range API specifies
that you can get an independent copy of a range is to use save, and generic
code should never rely on any other mechanism for that.

Now, ideally, we'd get rid of save and require that copying a forward range
result in an independent copy (which would then require that a class be
wrapped in a struct to be a range), but that's simply not how the current
range API works. Because the current range API does not make any guarantees
about the semantics of copying a range, generic code cannot assume that
those semantics are the same as save. As such, if you want an independent
copy of a forward range in generic code, you must use save, or the code will
be buggy. Similarly, generic code cannot use the original range after it's
been copied (unless it simply never does anything with the copy), because
mutating the copy may or may not mutate the original, and the original may
or may not even be in a valid state if the copy is mutated.

With non-generic code, you can rely on the behaviors of specific ranges and
what will happen when you copy them (e.g. not bothering to call save when
passing strings around), but with generic code, that's not true. And there's
plenty of D code out there that works correctly with a specific range type
but which would fail miserably if it were used generically, because it makes
assumption which the range API does not guarantee (like the semantics of
copying a range).

- Jonathan M Davis