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
next sibling 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
parent 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.

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
prev sibling 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*

       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 "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.

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