www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - How to change DList elements by means of foreach?

reply Alexandr Druzhinin <drug2004 bk.ru> writes:
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?
Sep 10 2012
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
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.
Sep 10 2012
next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
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*

Sep 10 2012
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
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
Sep 10 2012
prev sibling parent reply Alexandr Druzhinin <drug2004 bk.ru> writes:
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?
Sep 10 2012
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
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.
Sep 10 2012
parent reply "Brad Anderson" <eco gnuk.net> writes:
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?
Aug 20 2013
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
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.
Aug 20 2013
parent "Brad Anderson" <eco gnuk.net> writes:
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.
Aug 21 2013