www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Must ranges support `r1 = r2;`?

reply ag0aep6g <anonymous example.com> writes:
Long version: <https://issues.dlang.org/show_bug.cgi?id=18657> 
("std.range and std.algorithm can't handle refRange").

Short version: With two `std.range.RefRange`s, `r1 = r2;` does not what 
other Phobos code expects.

Question is: Who's at fault? Where do I fix this? Do ranges have to 
support assignment as expected - even though std.range doesn't mention 
it? Or should range-handling code never do that - even though it comes 
naturally and is widespread currently?
Mar 24 2018
next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Mar 24, 2018 at 10:44:35PM +0100, ag0aep6g via Digitalmars-d wrote:

 Long version: <https://issues.dlang.org/show_bug.cgi?id=18657>
 ("std.range and std.algorithm can't handle refRange").
 
 Short version: With two `std.range.RefRange`s, `r1 = r2;` does not
 what other Phobos code expects.
 
 Question is: Who's at fault? Where do I fix this? Do ranges have to
 support assignment as expected - even though std.range doesn't mention
 it? Or should range-handling code never do that - even though it comes
 naturally and is widespread currently?


Short answer: if r1=r2 is meant to save the current position in the
range, that's wrong; it should be r1=r2.save instead.

Long answer: I'm on my phone and don't want to type too much, so I'll
leave the explanations to someone else. :-P


T

-- 
A linguistics professor was lecturing to his class one day. "In English," he
said, "A double negative forms a positive. In some languages, though, such as
Russian, a double negative is still a negative. However, there is no language
wherein a double positive can form a negative." A voice from the back of the
room piped up, "Yeah, yeah."
Mar 24 2018
parent ag0aep6g <anonymous example.com> writes:
On 03/24/2018 10:57 PM, H. S. Teoh wrote:

 On Sat, Mar 24, 2018 at 10:44:35PM +0100, ag0aep6g via Digitalmars-d wrote:


[...]

 Short version: With two `std.range.RefRange`s, `r1 = r2;` does not
 what other Phobos code expects.




[...]

 Short answer: if r1=r2 is meant to save the current position in the
 range, that's wrong; it should be r1=r2.save instead.


`r1 = r2.save;` has the exact same problem. See the linked Bugzilla 
issue for details.
Mar 24 2018
prev sibling next sibling parent reply Simen =?UTF-8?B?S2rDpnLDpXM=?= <simen.kjaras gmail.com> writes:
On Saturday, 24 March 2018 at 21:44:35 UTC, ag0aep6g wrote:

 Long version: <https://issues.dlang.org/show_bug.cgi?id=18657> 
 ("std.range and std.algorithm can't handle refRange").

 Short version: With two `std.range.RefRange`s, `r1 = r2;` does 
 not what other Phobos code expects.

 Question is: Who's at fault? Where do I fix this? Do ranges 
 have to support assignment as expected - even though std.range 
 doesn't mention it? Or should range-handling code never do that 
 - even though it comes naturally and is widespread currently?


It seems to me you've misunderstood what refRange actually does. 
 From bugzilla:

void main()
{
     import std.range;
     import std.stdio;

     string s = "foo";
     auto r = refRange(&s);

     auto r2 = r;
     r2 = r2.save;
         /* Surprising: Effectively just does `s = s;` (i.e., 
nothing). */

     r2.popFront();
     writeln(r); /* Surprising: Prints "oo". */
}

None of this is surprising. When you call r2.popFront(), it's 
being forwarded to s.popFront. That's what refRange does, it's 
what it's supposed to do, and it's the only thing it could 
sensibly do.

This is conceptually what refRange does:

struct RefRange(R) {
     R* innerRange;
     this(R* innerRange) {
         this.innerRange = innerRange;
     }
     void popFront() {
         innerRange.popFront();
     }
      property auto front() {
         return innerRange.front;
     }
      property bool empty() {
         return innerRange.empty;
     }
}

In other words:

unittest {
     import std.range : refRange;
     import std.algorithm.comparison : equal;
     import std.array : popFront;

     auto a = "foo";
     auto b = refRange(&a);

     // Popping off the refRange pops off the original range.
     b.popFront();
     assert(a == "oo");

     // Popping off the original range pops off the refRange.
     a.popFront();
     assert(b.equal("o"));

     // b.equal("o") above had to iterate over the range,
     // thereby calling popFront until empty() returned true,
     // so the range is now empty.
     assert(a == "");
     assert(b.equal(""));
}

So when you do this:

     string s = "foo";
     auto r = refRange(&s).group;
     writeln(r.save);

r.save is going to create a new Group range, which contains a 
RefRange, which ultimately points to s. writeln() then repeatedly 
calls popFront(). This mutates s, as specified above.

You do have a point in the bug report on this point: calling 
writeln(r.save) again, should not write [Tuple!(dchar, uint)('f', 
1)], but just [], since the underlying range is now empty. This 
is a bug.

The 'chain', 'choose', and 'cycle' examples are an effect of 
strings not being random access ranges. I'm uncertain if we 
should call this a bug, but I agree the behavior is unexpected, 
so we probably should.

The splitter example is of course a bug. The crash, that is. The 
expected behavior is that the first writeln prints [foo, ar], 
which it does, and that the second print [].

--
   Simen
Mar 24 2018
next sibling parent Simen =?UTF-8?B?S2rDpnLDpXM=?= <simen.kjaras gmail.com> writes:
On Saturday, 24 March 2018 at 22:36:55 UTC, Simen Kjærås wrote:

 struct RefRange(R) {
     R* innerRange;
     this(R* innerRange) {
         this.innerRange = innerRange;
     }
     void popFront() {
         innerRange.popFront();
     }
      property auto front() {
         return innerRange.front;
     }
      property bool empty() {
         return innerRange.empty;
     }
 }


Minor nitpick: these should of course be 
(*innerRange).popFront(), etc.

--
   Simen
Mar 24 2018
prev sibling parent ag0aep6g <anonymous example.com> writes:
On 03/24/2018 11:36 PM, Simen Kjærås wrote:

 void main()
 {
      import std.range;
      import std.stdio;
 
      string s = "foo";
      auto r = refRange(&s);
 
      auto r2 = r;
      r2 = r2.save;
          /* Surprising: Effectively just does `s = s;` (i.e., nothing).
*/
 
      r2.popFront();
      writeln(r); /* Surprising: Prints "oo". */
 }
 
 None of this is surprising. When you call r2.popFront(), it's being 
 forwarded to s.popFront. That's what refRange does, it's what it's 
 supposed to do, and it's the only thing it could sensibly do.


I think it's surprising. It works as intended and as documented, but 
it's surprising. Note that the behavior is different when you change

     auto r2 = r;
     r2 = r2.save;

to

     auto r2 = r.save;

`.save` actually makes a new slice that can be popped independently from 
`s`. But the assignment throws that independent slice away.


 This is conceptually what refRange does:
 
 struct RefRange(R) {
      R* innerRange;
      this(R* innerRange) {
          this.innerRange = innerRange;
      }
      void popFront() {
          innerRange.popFront();
      }
       property auto front() {
          return innerRange.front;
      }
       property bool empty() {
          return innerRange.empty;
      }
 }


That's all fine. You're missing the interesting bit: opAssign. `save` 
might also be interesting, but the actual `save` is perfectly fine.

[...]

 So when you do this:
 
      string s = "foo";
      auto r = refRange(&s).group;
      writeln(r.save);
 
 r.save is going to create a new Group range, which contains a RefRange, 
 which ultimately points to s. writeln() then repeatedly calls 
 popFront(). This mutates s, as specified above.


That's not how it should be. A saved range should be iterable 
independently from the original. That's the point of `.save`. If that's 
not possible, `.save` should not be there.

You're implying that a saved RefRange points to the same range as the 
original. That's not true. RefRange.save saves the underlying range and 
lets the new RefRange point to that. This is correct. The two ranges can 
be iterated independently.

It's inside `Group` that things get messed up. There you have this 
innocent looking implementation of `save` [1]:

      property typeof(this) save() {
         typeof(this) ret = this;
         ret._input = this._input.save;
         ret._current = this._current;
         return ret;
     }

The problem is in the line `ret._input = this._input.save;`. That calls 
RefRange.opAssign. If you refactor the code to avoid opAssign, saving 
works properly. For example:

     private this(typeof(_input) inp, typeof(_current) cur)
     {
         this._input = inp; /* Initialization, not assigment. */
         this._current = cur; /* Ditto, but doesn't matter. */
     }
      property typeof(this) save() {
         return typeof(this)(this._input.save, this._current);
     }

Note that it calls `_input.save` just the same as before. The code only 
avoids RefRange.opAssign.

[...]

 The 'chain', 'choose', and 'cycle' examples are an effect of strings not 
 being random access ranges. I'm uncertain if we should call this a bug, 
 but I agree the behavior is unexpected, so we probably should.


I don't think random access has anything to do with this.


 The splitter example is of course a bug. The crash, that is. The 
 expected behavior is that the first writeln prints [foo, ar], which it 
 does, and that the second print [].


Again, I don't agree. You're effectively saying that saving a RefRange 
isn't possible. If that were so, it simply shouldn't have a `save` 
method. But the `save` method works perfectly fine. It's Phobos 
(accidentally) calling RefRange.opAssign that breaks things.


[1] 
https://github.com/dlang/phobos/blob/3225b19c9bae4b7e8d7a93d2d8c22c94b2a1a6c5/std/algorithm/iteration.d#L1505-L1510
Mar 24 2018
prev sibling next sibling parent reply Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Saturday, March 24, 2018 22:44:35 ag0aep6g via Digitalmars-d wrote:

 Long version: <https://issues.dlang.org/show_bug.cgi?id=18657>
 ("std.range and std.algorithm can't handle refRange").

 Short version: With two `std.range.RefRange`s, `r1 = r2;` does not what
 other Phobos code expects.

 Question is: Who's at fault? Where do I fix this? Do ranges have to
 support assignment as expected - even though std.range doesn't mention
 it? Or should range-handling code never do that - even though it comes
 naturally and is widespread currently?


I'll have to sit down and study RefRange again to really say whether it's
violating how ranges are supposed to work, but the key idea behind RefRange
is that it makes it possible to pass a range to a function and not have it
be copied in the process so that if you have something like

void foo(R)(R range)
{
    ...
}

you can pass it a range and continue to use what's left of the range after
it's done. As far as copying and assignment go, when you have

auto range2 = range1;

as far as generic code is concerned, it's effectively undefined behavior to
then do anything with range1. The same goes with passing a range to a
function by value or doing anything else that copies a range. The behavior
of how ranges in general work is not defined for copying.

If the range is a reference type, then range1 and range2 refer to each
other, and attempting to iterate either range1 or range2 would iterate the
other.

If the range is a value type, then range1 and range2 are then independent,
and iterating one does not affect the other.

If the range is a pseudo-reference type, then what happens to one when
trying to iterate the other depends entirely on the implementation. In the
case of dynamic arrays, you get the same behavior as a value type, but in
many other cases, you get a weird halfway state, because calling popFront on
range2 affects _some_ of the state in range1 but not all.

As such, in generic code, the only operation that I would expect to be valid
on range1 after

auto range2 = range1;

would be to assign range1 another value - be that range2 or another range of
the same type, at which point, range1 should then be able to be iterated
while the range that was assigned to it can't be, because it then would have
been copied, and the behavior for using it would be undefined. e.g.

auto range2 = range1; // now, range1 can't be used until it's assigned to
range2.popFront();

range1 = range2; // now, range2 can't be used until it's assigned to
range1.popFront();

And I don't think that RefRange violates that. Regardless, any generic,
range-based code that uses a range after copying it (and I mean actually
copying it, not calling save) is buggy code, and it _can't_ work correctly
because of how diverse the behavior of copying ranges is. Any generic, code
that copies a range and then uses it is wrong. Non-generic, range-based code
can do it, because the type of the range is well-known and thus the
semantics of copying are well-known and can be depended on, but taht's not
the case for generic, range-based code (which most of the range-based code
in Phobos is).

As far as save goes, save is supposed to create an independent copy to
iterate, and RefRange's documentation says that that's what it does, meaning
that passing a RefRange to a function that uses the forward range API
probably isn't very useful, but it would do what a forward range is supposed
to do. Whether the actual implementation does that properly on not, I don't
know - I'd have to study it - but per the documentation, it's at least
designed to do the right thing with save.

- Jonathan M Davis
Mar 24 2018
parent reply ag0aep6g <anonymous example.com> writes:
On 03/25/2018 12:02 AM, Jonathan M Davis wrote:

 auto range2 = range1; // now, range1 can't be used until it's assigned to
 range2.popFront();
 
 range1 = range2; // now, range2 can't be used until it's assigned to
 range1.popFront();
 
 And I don't think that RefRange violates that.


What RefRange violates is the assumption that range1 gets discarded, and 
that it simply gets overwritten by range2.

Note that `auto range2 = range1;` does not have that problem, because 
it's initialization, not assignment. It doesn't call RefRange's funky 
opAssign.

The various misbehaving Phobos functions assume that assignment works 
the same as initialization. But it doesn't with RefRange.

[...]

 As far as save goes, save is supposed to create an independent copy to
 iterate, and RefRange's documentation says that that's what it does, meaning
 that passing a RefRange to a function that uses the forward range API
 probably isn't very useful, but it would do what a forward range is supposed
 to do. Whether the actual implementation does that properly on not, I don't
 know - I'd have to study it - but per the documentation, it's at least
 designed to do the right thing with save.


RefRange.save works correctly. opAssign is the problem.
Mar 24 2018
parent reply Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Sunday, March 25, 2018 00:28:32 ag0aep6g via Digitalmars-d wrote:

 On 03/25/2018 12:02 AM, Jonathan M Davis wrote:

 auto range2 = range1; // now, range1 can't be used until it's assigned
 to
 range2.popFront();

 range1 = range2; // now, range2 can't be used until it's assigned to
 range1.popFront();

 And I don't think that RefRange violates that.


 What RefRange violates is the assumption that range1 gets discarded, and
 that it simply gets overwritten by range2.

 Note that `auto range2 = range1;` does not have that problem, because
 it's initialization, not assignment. It doesn't call RefRange's funky
 opAssign.

 The various misbehaving Phobos functions assume that assignment works
 the same as initialization. But it doesn't with RefRange.


I'll have to think about it. IIRC, for RefRange to work as it's supposed to,
opAssign really does need to work the way that it does, but it's been a
while since I did much with RefRange, so I don't know. Either way,
assignment really isn't part of the range API. I don't think that any of the
range traits even test that assignment works on any level. So, it could be
argued that generic, range-based functions simply shouldn't ever be
assigning one range to another. I'm pretty sure that technically, a range
could  disable opAssign without violating the range API, though it wouldn't
surprise me if such a range then didn't work with a lot of existing code.

On some level, this falls into the same trap as save in that _usually_ save
is not required, but in some cases it is, so it's frequently the case that
save gets skipped when it's supposed to be called, and it only gets caught
when tests are added which use ranges which are reference types.

Similarly, it's quite common for range-based functions to assume that front
is not transitive, but occasionally, front is transitive, which then causes
problems. It's been argued that the solution for that is that any generic
code which retains the result of front after popFront is called needs to
call save before getting front in order to guarantee that the result of
front is independent, but doing that is not common practice at all and has
been debated on a number of occasions.

RefRange is operating in an area where the range API doesn't actually define
the semantics - either in the traits themselves or the behavior that's
documented as expected but not actually guaranteed by the traits (e.g. as is
the case with save - it's behavior can't be enforced, just documented).

So, I think that there's a good argument to be made that truly generic
range-based code should not be assigning one range to another, because the
range API does not actually require that ranges even support assignment, but
it also doesn't surprise me at all if folks do it quite a bit. We frequently
have the problem with ranges that folks assume certain behaviors based on
what the typical range does but which is not actually guaranteed by the
range API. And I don't know what the solution is for that. Certainly, the
result is that a lot of range-based code that exists doesn't actually work
with any range that matches the template constraints. Phobos does a far
better job of it than a lot of code, because it's written to be generic and
has lots of tests to catch corner cases that many folks never test for, but
we don't catch everything.

A lot of stuff would be cleaner if we could somehow require that all basic
input ranges be reference types and all forward ranges be value types (which
would include eliminating save from the range API), but that would be overly
restrictive in some cases, and it would be too disruptive a change now. And
for better or worse, RefRange probably wouldn't into that more restrictive
paradigm.

- Jonathan M Davis
Mar 24 2018
parent ag0aep6g <anonymous example.com> writes:
On 03/25/2018 01:49 AM, Jonathan M Davis wrote:

 assignment really isn't part of the range API. I don't think that any of the
 range traits even test that assignment works on any level. So, it could be
 argued that generic, range-based functions simply shouldn't ever be
 assigning one range to another.


So we have to remove all range assignments from generic code. Ugh.

The first three: https://github.com/dlang/phobos/pull/6346
Mar 25 2018
prev sibling parent reply Jack Stouffer <jack jackstouffer.com> writes:
On Saturday, 24 March 2018 at 21:44:35 UTC, ag0aep6g wrote:

 Long version: <https://issues.dlang.org/show_bug.cgi?id=18657> 
 ("std.range and std.algorithm can't handle refRange").

 Short version: With two `std.range.RefRange`s, `r1 = r2;` does 
 not what other Phobos code expects.

 Question is: Who's at fault? Where do I fix this? Do ranges 
 have to support assignment as expected - even though std.range 
 doesn't mention it? Or should range-handling code never do that 
 - even though it comes naturally and is widespread currently?


Posting this from the PR.

This is a band-aid over the issue of what to do with funky 
operator overloads in ranges. In conjunction with this issue, you 
also have to assume that one could do stuff in the ctor that 
would mess up existing range code in Phobos. What really needs to 
happen is the range spec needs to be updated with a clear list of 
assumptions that Phobos makes in regards to these functions. I'm 
pretty sure that RefRange would have been designed differently if 
the designer realized the results of overloading opAssign like 
that.

In a perfect world, we'd have a pre-constructed test suite that 
people could plug their range (with some data in it) into, where 
all the tests make sure all Phobos assumptions hold true.

In my opinion, it would be best to update the range spec and go 
from there.
Apr 16 2018
next sibling parent reply "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Mon, Apr 16, 2018 at 03:58:34PM +0000, Jack Stouffer via Digitalmars-d wrote:
[...]

 In a perfect world, we'd have a pre-constructed test suite that people
 could plug their range (with some data in it) into, where all the
 tests make sure all Phobos assumptions hold true.


[...]

We could perhaps add a little test framework in std.internal.*
containing tests that run through a variety of different types of
ranges.  I think there's already something like this somewhere, it's
just not used as widely as it could be.

I could come up with some pretty pathological range types for
stress-testing, if you want.  (These are actual range types I've used in
my own code, I didn't set out to break Phobos, but it turned out that
Phobos makes many unstated assumptions that explode in your face when
given a more unusual instance of a range. There used to be (and may
still be) a lot of places that essentially assumes built-in array
semantics, and that blew up in ugly ways when handed something else
that's nevertheless still a valid range.)


T

-- 
Making non-nullable pointers is just plugging one hole in a cheese grater. --
Walter Bright
Apr 16 2018
parent Jack Stouffer <jack jackstouffer.com> writes:
On Monday, 16 April 2018 at 16:15:30 UTC, H. S. Teoh wrote:

 I could come up with some pretty pathological range types for 
 stress-testing, if you want.  (These are actual range types 
 I've used in my own code, I didn't set out to break Phobos, but 
 it turned out that Phobos makes many unstated assumptions that 
 explode in your face when given a more unusual instance of a 
 range. There used to be (and may still be) a lot of places that 
 essentially assumes built-in array semantics, and that blew up 
 in ugly ways when handed something else that's nevertheless 
 still a valid range.)


It would be awesome to have these as range types in 
std.internal.test.dummyrange. Then they could be used in tests 
throughout Phobos.
Apr 16 2018
prev sibling parent reply Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Monday, April 16, 2018 15:58:34 Jack Stouffer via Digitalmars-d wrote:

 On Saturday, 24 March 2018 at 21:44:35 UTC, ag0aep6g wrote:

 Long version: <https://issues.dlang.org/show_bug.cgi?id=18657>
 ("std.range and std.algorithm can't handle refRange").

 Short version: With two `std.range.RefRange`s, `r1 = r2;` does
 not what other Phobos code expects.

 Question is: Who's at fault? Where do I fix this? Do ranges
 have to support assignment as expected - even though std.range
 doesn't mention it? Or should range-handling code never do that
 - even though it comes naturally and is widespread currently?


 Posting this from the PR.

 This is a band-aid over the issue of what to do with funky
 operator overloads in ranges. In conjunction with this issue, you
 also have to assume that one could do stuff in the ctor that
 would mess up existing range code in Phobos. What really needs to
 happen is the range spec needs to be updated with a clear list of
 assumptions that Phobos makes in regards to these functions. I'm
 pretty sure that RefRange would have been designed differently if
 the designer realized the results of overloading opAssign like
 that.

 In a perfect world, we'd have a pre-constructed test suite that
 people could plug their range (with some data in it) into, where
 all the tests make sure all Phobos assumptions hold true.

 In my opinion, it would be best to update the range spec and go
 from there.


Well, the reality of the matter is that if RefRange's opAssign doesn't work
the way that it works, then RefRange is broken and fails at its purpose (and
this is the designer of it speaking). So, if RefRange couldn't do what it's
doing, it wouldn't exist, and no requirements have ever been place on
opAssign for ranges (in fact, for better or worse, the range API doesn't
actually require that opAssign even work for ranges, though it can certainly
be argued that it should). Now, RefRange is weird enough overall that maybe
it shouldn't exist, and it was trying to solve a problem that's not really
solvable in the general case (basically it's trying to force a range-based
function to mutate the original rather than to deal with a copy even though
the function was written to copy) - especially once forward ranges get
involved - but opAssign is doing exactly what it was intended to do, and if
what it's doing wasn't considered valid at the time, RefRange wouldn't
exist. Either way, there are times when I think that adding RefRange was a
mistake precisely because it's such an oddball and because it's only ever
really going to do what it was intended to do in some circumstances.

- Jonathan M Davis
Apr 16 2018
parent reply Jack Stouffer <jack jackstouffer.com> writes:
On Monday, 16 April 2018 at 16:22:04 UTC, Jonathan M Davis wrote:

 Well, the reality of the matter is that if RefRange's opAssign 
 doesn't work the way that it works, then RefRange is broken and 
 fails at its purpose (and this is the designer of it speaking). 
 So, if RefRange couldn't do what it's doing, it wouldn't exist, 
 and no requirements have ever been place on opAssign for ranges 
 (in fact, for better or worse, the range API doesn't actually 
 require that opAssign even work for ranges, though it can 
 certainly be argued that it should). Now, RefRange is weird 
 enough overall that maybe it shouldn't exist, and it was trying 
 to solve a problem that's not really solvable in the general 
 case (basically it's trying to force a range-based function to 
 mutate the original rather than to deal with a copy even though 
 the function was written to copy) - especially once forward 
 ranges get involved - but opAssign is doing exactly what it was 
 intended to do, and if what it's doing wasn't considered valid 
 at the time, RefRange wouldn't exist. Either way, there are 
 times when I think that adding RefRange was a mistake precisely 
 because it's such an oddball and because it's only ever really 
 going to do what it was intended to do in some circumstances.


RefRange can be useful in certain situations. But, I don't see a 
pretty way out of this. Either

1. we break code by saying `RefRange` is doing something illegal
2. we break code by making `RefRange` always an input range
3. we change all range code which could use `opAssign` to use 
`move`, potentially breaking code which relies on opAssign being 
called, and imposing a large burden on the maintenance of 
existing code for a very small corner case. Just imagine how much 
code outside of Phobos also has this subtle bug.
Apr 16 2018
parent Jonathan M Davis <newsgroup.d jmdavisprog.com> writes:
On Monday, April 16, 2018 19:01:02 Jack Stouffer via Digitalmars-d wrote:

 On Monday, 16 April 2018 at 16:22:04 UTC, Jonathan M Davis wrote:

 Well, the reality of the matter is that if RefRange's opAssign
 doesn't work the way that it works, then RefRange is broken and
 fails at its purpose (and this is the designer of it speaking).
 So, if RefRange couldn't do what it's doing, it wouldn't exist,
 and no requirements have ever been place on opAssign for ranges
 (in fact, for better or worse, the range API doesn't actually
 require that opAssign even work for ranges, though it can
 certainly be argued that it should). Now, RefRange is weird
 enough overall that maybe it shouldn't exist, and it was trying
 to solve a problem that's not really solvable in the general
 case (basically it's trying to force a range-based function to
 mutate the original rather than to deal with a copy even though
 the function was written to copy) - especially once forward
 ranges get involved - but opAssign is doing exactly what it was
 intended to do, and if what it's doing wasn't considered valid
 at the time, RefRange wouldn't exist. Either way, there are
 times when I think that adding RefRange was a mistake precisely
 because it's such an oddball and because it's only ever really
 going to do what it was intended to do in some circumstances.


 RefRange can be useful in certain situations. But, I don't see a
 pretty way out of this. Either

 1. we break code by saying `RefRange` is doing something illegal
 2. we break code by making `RefRange` always an input range
 3. we change all range code which could use `opAssign` to use
 `move`, potentially breaking code which relies on opAssign being
 called, and imposing a large burden on the maintenance of
 existing code for a very small corner case. Just imagine how much
 code outside of Phobos also has this subtle bug.


Honestly, I'm perfectly fine with RefRange only working some of the time.
Even if the function doesn't use assignment anywhere, RefRange doesn't
necessarily really work correctly with regards to its goal anyway in the
sense that as soon as save is used, you lose any changes that are made. And
even if RefRange doesn't work with some range-based functions that doesn't
mean that it can't work with others. We can simply go the route of putting a
note on RefRange that it may not work correctly with all range-based
algorithms - that it's there for the cases where it's useful but that the
nature of its semantics make it so that it's never going to work well with
some functions.

As for changing range-based code that uses assignment, if it really needs
assignment, then I'd favor just saying that RefRange doesn't work properly
with it and tough luck. It's not like it's ever worked properly with such a
function. But at the same time, I don't know why a function would _need_
assignment to work with ranges. Typically, you can just create a new
variable, and you're fine. It wouldn't surprise me if there's some example
where assignment would be needed, but I don't think that it's typical. So,
if a range-based function can easily avoid using assignment on ranges, it
probably should.

Part of the problem here is that when you're dealing with generic code, you
can't make very many assumptions, but it's so, so easy to make assumptions
that are almost alwayws true that it becomes almost impossible to avoid
making such assumptions at least from time to time even if you don't mean
to. Locking down the required semantics even further can help by
guaranteeing that more assumptions are valid, but that also eliminates
certain use cases (ranges with transitive front would be a good example of
this; ideally, no such ranges would exist, because they cause major
problems, but at the same time, they can sometimes be really useful -
particularly when performance is important). So, I suspect that we're going
to be stuck finding problems like this from time to time for years to come,
though hopefully only rarely.

- Jonathan M Davis
Apr 16 2018