Thread overview
How to change DList elements by means of foreach?
Sep 10, 2012
Alexandr Druzhinin
Sep 10, 2012
monarch_dodra
Sep 10, 2012
monarch_dodra
Sep 10, 2012
Jonathan M Davis
Sep 10, 2012
Alexandr Druzhinin
Sep 10, 2012
monarch_dodra
Aug 21, 2013
Brad Anderson
Aug 21, 2013
monarch_dodra
Aug 21, 2013
Brad Anderson
September 10, 2012
I have code:

import std.container;

int main() {
// array
int[] array = [0];
foreach(ref value; array) {
	value += 50;
	assert(value == 50);
}

foreach(value; array) {
	assert(value == 50);
}	

// double-linked list;
DList!int dlist;
dlist.insertFront(0);
foreach(ref value; dlist) {
	value += 50;
	assert(value == 50);
}

foreach(value; dlist) {
	assert(value == 50);  // Why do I have assertion failure here?
}

}
How to change the value of elements of DList?
September 10, 2012
On Monday, 10 September 2012 at 11:18:29 UTC, Alexandr Druzhinin wrote:
> I have code:
>
> import std.container;
>
> int main() {
> // array
> int[] array = [0];
> foreach(ref value; array) {
> 	value += 50;
> 	assert(value == 50);
> }
>
> foreach(value; array) {
> 	assert(value == 50);
> }	
>
> // double-linked list;
> DList!int dlist;
> dlist.insertFront(0);
> foreach(ref value; dlist) {
> 	value += 50;
> 	assert(value == 50);
> }
>
> foreach(value; dlist) {
> 	assert(value == 50);  // Why do I have assertion failure here?
> }
>
> }
> How to change the value of elements of DList?

There is a know bug: foreach with ref does not currently work these containers. The reason is that the container's front does not actually expose a reference, but a value, and that is what is being changed (the returned value).

There is no hope in sight to really *ever* make it work, because "container.front += 5" can't be made to work if the returned value is not a reference: Unlike indexes that define opIndexOpAssign, there is no opFrontOpAssign.

What bothers *me* though is that the code compiles fine, biting more than 1 user in the process.

Anyways... the workaround is* making an explicit loop, with temporary object that is fed back into front, like this:

import std.container;

--------
void main()
{
    // double-linked list;
    DList!int dlist;
    dlist.insertFront(0);
    auto slice = dlist[]; //Extract a range manually
    for( ; !slice.empty ; slice.popFront() )
    {
      auto value = slice.front; //Extract the value
      value += 50;              //Increment the value
      slice.front() = value;    //Feed back into the range*
    }

    foreach(value; dlist) {
      assert(value == 50);  //Now this works fine
    }
}
--------

Well... this *would* work, but apparently, the implementation of DList.Range doesn't define front(T value). This makes the Range pretty much read-only. My guess is that this was an omission on the part of the implementer. I will fix it so that it works.


September 10, 2012
On Monday, 10 September 2012 at 11:36:42 UTC, monarch_dodra wrote:
>       slice.front() = value;    //Feed back into the range*
Typo in my code when I was investigating, this should be:
>       slice.front = value;      //Feed back into the range*

September 10, 2012
On Monday, September 10, 2012 13:37:15 monarch_dodra wrote:
> What bothers *me* though is that the code compiles fine, biting more than 1 user in the process.

Which is definitely a bug. If it hasn't been reported, it should be (I suspect that it has, but I don't know for sure and don't want to go digging for it at the moment).

- Jonathan M Davis
September 10, 2012
10.09.2012 18:37, monarch_dodra пишет:
>
> There is a know bug: foreach with ref does not currently work these
> containers. The reason is that the container's front does not actually
> expose a reference, but a value, and that is what is being changed (the
> returned value).
>
> There is no hope in sight to really *ever* make it work, because
> "container.front += 5" can't be made to work if the returned value is
> not a reference: Unlike indexes that define opIndexOpAssign, there is no
> opFrontOpAssign.
>
> What bothers *me* though is that the code compiles fine, biting more
> than 1 user in the process.
>
> Anyways... the workaround is* making an explicit loop, with temporary
> object that is fed back into front, like this:
>
> import std.container;
>
> --------
> void main()
> {
>      // double-linked list;
>      DList!int dlist;
>      dlist.insertFront(0);
>      auto slice = dlist[]; //Extract a range manually
>      for( ; !slice.empty ; slice.popFront() )
>      {
>        auto value = slice.front; //Extract the value
>        value += 50;              //Increment the value
>        slice.front() = value;    //Feed back into the range*
>      }
>
>      foreach(value; dlist) {
>        assert(value == 50);  //Now this works fine
>      }
> }
> --------
>
> Well... this *would* work, but apparently, the implementation of
> DList.Range doesn't define front(T value). This makes the Range pretty
> much read-only. My guess is that this was an omission on the part of the
> implementer. I will fix it so that it works.
>
>
Good to know, but bad to do...

If in std.container:
1553:        @property T front() { return _first._payload; }
change to:
1553:        @property *ref* T front() { return _first._payload; }
doesn't it solve the problem or I don't know/understand something else?
September 10, 2012
On Monday, 10 September 2012 at 12:44:36 UTC, Alexandr Druzhinin wrote:
> 10.09.2012 18:37, monarch_dodra пишет:
>>
>> There is a know bug: foreach with ref does not currently work these
>> containers. The reason is that the container's front does not actually
>> expose a reference, but a value, and that is what is being changed (the
>> returned value).
>>
>> There is no hope in sight to really *ever* make it work, because
>> "container.front += 5" can't be made to work if the returned value is
>> not a reference: Unlike indexes that define opIndexOpAssign, there is no
>> opFrontOpAssign.
>>
>> What bothers *me* though is that the code compiles fine, biting more
>> than 1 user in the process.
>>
>> Anyways... the workaround is* making an explicit loop, with temporary
>> object that is fed back into front, like this:
>>
>> import std.container;
>>
>> --------
>> void main()
>> {
>>     // double-linked list;
>>     DList!int dlist;
>>     dlist.insertFront(0);
>>     auto slice = dlist[]; //Extract a range manually
>>     for( ; !slice.empty ; slice.popFront() )
>>     {
>>       auto value = slice.front; //Extract the value
>>       value += 50;              //Increment the value
>>       slice.front() = value;    //Feed back into the range*
>>     }
>>
>>     foreach(value; dlist) {
>>       assert(value == 50);  //Now this works fine
>>     }
>> }
>> --------
>>
>> Well... this *would* work, but apparently, the implementation of
>> DList.Range doesn't define front(T value). This makes the Range pretty
>> much read-only. My guess is that this was an omission on the part of the
>> implementer. I will fix it so that it works.
>>
>>
> Good to know, but bad to do...
>
> If in std.container:
> 1553:        @property T front() { return _first._payload; }
> change to:
> 1553:        @property *ref* T front() { return _first._payload; }
> doesn't it solve the problem or I don't know/understand something else?

Arguably yes, however, the idea is that a container is supposed to have an implementation defined allocator, meaning that operations "may or may mot" invalidate references. So it is not allowed to return a reference.

IMO, this is a valid argument for things like Array, that "can and will" move objects around, without ever telling the accessing ranges. Giving reference access here would be most dangerous. Not impossible, but very unsafe, and Phobos strives to be safe.
The same argument applies to BinaryHeap, which is just a container adaptor.

However, for any "node" based structure (such as {SD}List), which are structures that users usually chose *because* references are *always* valid, the argument doesn't hold as well. In particular, even with an implementation defined allocator, there is no reason a reference can't be returned. I'll try to push for reference access, but I may be turned down on the simple argument of "container uniformity" :/

Finally, regarding RedBlackTree, technically, you shouldn't assign to a node in the tree, but rather remove re-insert, so that is a non-issue.
August 21, 2013
On Monday, 10 September 2012 at 13:05:16 UTC, monarch_dodra wrote:
> On Monday, 10 September 2012 at 12:44:36 UTC, Alexandr Druzhinin wrote:
>> 10.09.2012 18:37, monarch_dodra пишет:
>>>
>>> There is a know bug: foreach with ref does not currently work these
>>> containers. The reason is that the container's front does not actually
>>> expose a reference, but a value, and that is what is being changed (the
>>> returned value).
>>>
>>> There is no hope in sight to really *ever* make it work, because
>>> "container.front += 5" can't be made to work if the returned value is
>>> not a reference: Unlike indexes that define opIndexOpAssign, there is no
>>> opFrontOpAssign.
>>>
>>> What bothers *me* though is that the code compiles fine, biting more
>>> than 1 user in the process.
>>>
>>> Anyways... the workaround is* making an explicit loop, with temporary
>>> object that is fed back into front, like this:
>>>
>>> import std.container;
>>>
>>> --------
>>> void main()
>>> {
>>>    // double-linked list;
>>>    DList!int dlist;
>>>    dlist.insertFront(0);
>>>    auto slice = dlist[]; //Extract a range manually
>>>    for( ; !slice.empty ; slice.popFront() )
>>>    {
>>>      auto value = slice.front; //Extract the value
>>>      value += 50;              //Increment the value
>>>      slice.front() = value;    //Feed back into the range*
>>>    }
>>>
>>>    foreach(value; dlist) {
>>>      assert(value == 50);  //Now this works fine
>>>    }
>>> }
>>> --------
>>>
>>> Well... this *would* work, but apparently, the implementation of
>>> DList.Range doesn't define front(T value). This makes the Range pretty
>>> much read-only. My guess is that this was an omission on the part of the
>>> implementer. I will fix it so that it works.
>>>
>>>
>> Good to know, but bad to do...
>>
>> If in std.container:
>> 1553:        @property T front() { return _first._payload; }
>> change to:
>> 1553:        @property *ref* T front() { return _first._payload; }
>> doesn't it solve the problem or I don't know/understand something else?
>
> Arguably yes, however, the idea is that a container is supposed to have an implementation defined allocator, meaning that operations "may or may mot" invalidate references. So it is not allowed to return a reference.
>
> IMO, this is a valid argument for things like Array, that "can and will" move objects around, without ever telling the accessing ranges. Giving reference access here would be most dangerous. Not impossible, but very unsafe, and Phobos strives to be safe.
> The same argument applies to BinaryHeap, which is just a container adaptor.
>
> However, for any "node" based structure (such as {SD}List), which are structures that users usually chose *because* references are *always* valid, the argument doesn't hold as well. In particular, even with an implementation defined allocator, there is no reason a reference can't be returned. I'll try to push for reference access, but I may be turned down on the simple argument of "container uniformity" :/
>
> Finally, regarding RedBlackTree, technically, you shouldn't assign to a node in the tree, but rather remove re-insert, so that is a non-issue.

Has anything happened since this question was asked that moves toward fixes this?  I just hit it myself with std.container.Array.  How is anyone supposed to do anything productive with std.container containers with this glaring limitation?  I'm kind of surprised std.container is even included with phobos with such a deficiency. Does anyone actually use these containers?
August 21, 2013
On Wednesday, 21 August 2013 at 03:15:14 UTC, Brad Anderson wrote:
> Has anything happened since this question was asked that moves toward fixes this?  I just hit it myself with std.container.Array.  How is anyone supposed to do anything productive with std.container containers with this glaring limitation?  I'm kind of surprised std.container is even included with phobos with such a deficiency. Does anyone actually use these containers?

Regarding the foreach and reference problems, AFAIK, no, nothing has been done.

I *have* been fixing more serious problems with DList (the list simply corrupts itself under certain situations). My last pull is currently stuck in "LGTM, but not pulled" limbo.

Once it DList's "Integrity" is fixed, I'll push to update it to use references.
August 21, 2013
On Wednesday, 21 August 2013 at 06:07:21 UTC, monarch_dodra wrote:
> On Wednesday, 21 August 2013 at 03:15:14 UTC, Brad Anderson wrote:
>> Has anything happened since this question was asked that moves toward fixes this?  I just hit it myself with std.container.Array.  How is anyone supposed to do anything productive with std.container containers with this glaring limitation?  I'm kind of surprised std.container is even included with phobos with such a deficiency. Does anyone actually use these containers?
>
> Regarding the foreach and reference problems, AFAIK, no, nothing has been done.
>
> I *have* been fixing more serious problems with DList (the list simply corrupts itself under certain situations). My last pull is currently stuck in "LGTM, but not pulled" limbo.
>
> Once it DList's "Integrity" is fixed, I'll push to update it to use references.

That'd be great.  I'm not sure if you can detect SafeD during compilation but perhaps not allowing references when SafeD is enabled would be a good compromise.