Re: Must ranges support `r1 = r2;`?

2018-04-16 Thread Jonathan M Davis via Digitalmars-d
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



Re: Must ranges support `r1 = r2;`?

2018-04-16 Thread Jack Stouffer via Digitalmars-d

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.


Re: Must ranges support `r1 = r2;`?

2018-04-16 Thread Jack Stouffer via Digitalmars-d

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.




Re: Must ranges support `r1 = r2;`?

2018-04-16 Thread Jonathan M Davis via Digitalmars-d
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: 
> > ("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



Re: Must ranges support `r1 = r2;`?

2018-04-16 Thread H. S. Teoh via Digitalmars-d
On Mon, Apr 16, 2018 at 03:58:34PM +, 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


Re: Must ranges support `r1 = r2;`?

2018-04-16 Thread Jack Stouffer via Digitalmars-d

On Saturday, 24 March 2018 at 21:44:35 UTC, ag0aep6g wrote:
Long version:  
("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.


Re: Must ranges support `r1 = r2;`?

2018-03-25 Thread ag0aep6g via Digitalmars-d

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


Re: Must ranges support `r1 = r2;`?

2018-03-24 Thread Jonathan M Davis via Digitalmars-d
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



Re: Must ranges support `r1 = r2;`?

2018-03-24 Thread ag0aep6g via Digitalmars-d

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.


Re: Must ranges support `r1 = r2;`?

2018-03-24 Thread ag0aep6g via Digitalmars-d

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();

     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().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


Re: Must ranges support `r1 = r2;`?

2018-03-24 Thread Jonathan M Davis via Digitalmars-d
On Saturday, March 24, 2018 22:44:35 ag0aep6g via Digitalmars-d wrote:
> Long version: 
> ("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



Re: Must ranges support `r1 = r2;`?

2018-03-24 Thread Simen Kjærås via Digitalmars-d

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


Re: Must ranges support `r1 = r2;`?

2018-03-24 Thread Simen Kjærås via Digitalmars-d

On Saturday, 24 March 2018 at 21:44:35 UTC, ag0aep6g wrote:
Long version:  
("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();

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();

// 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().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




Re: Must ranges support `r1 = r2;`?

2018-03-24 Thread ag0aep6g via Digitalmars-d

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.


Re: Must ranges support `r1 = r2;`?

2018-03-24 Thread H. S. Teoh via Digitalmars-d
On Sat, Mar 24, 2018 at 10:44:35PM +0100, ag0aep6g via Digitalmars-d wrote:
> Long version: 
> ("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."


Must ranges support `r1 = r2;`?

2018-03-24 Thread ag0aep6g via Digitalmars-d
Long version:  
("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?