Thread overview | |||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
July 02, 2012 foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
I think this is a pretty serious bug: when one writes: "foreach(ref a, range)", the underlying (ref'd) object will ONLY get modified if the range object provides a "ref T front()" method. What is worrisome is that: a) The code compiles anyways. b) This even happens when range provides a "void front(T t)" method. Here is the bug in action, with the very standard Array class, as well as the generic Map algorithm: ---- import std.container; import std.stdio; import std.algorithm; void main() { Array!int arr; arr.length = 3; foreach(a; arr) a = 2; map!("a+=2")(arr[]); writeln(arr[]); } ---- Output: ---- [0, 0, 0] ---- As you can see, not only is "foreach" broken, but so is any algorithm that tries to mutate an Array. Note that "Array" itself can be fixed by my recommendation here: http://forum.dlang.org/thread/bkozswmsgeibarowfwvq@forum.dlang.org However, in the case of containers that can't return a ref'ed front (eg Array!bool), this is actually a double bug: 1) foreach: It makes compile time call to "T front()" regardless of context. Because of this, even when we write "a = 2", a straight up assignment to a temporary takes place, rather than compiling as "arr.front(2)". 2) front: Unlike opIndex, front is not an operator. This means that writing code such as "arr.front += 5" or "++arr.front" is not supported by the language (when front returns by value). This means the implementer of a Range (which can't return a ref'd front) has absolutely no way of intercepting operations that want to occur directly on front |
July 02, 2012 Re: foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
Posted in reply to monarch_dodra | On Monday, 2 July 2012 at 12:44:59 UTC, monarch_dodra wrote:
> I think this is a pretty serious bug: when one writes: "foreach(ref a, range)", the underlying (ref'd) object will ONLY get modified if the range object provides a "ref T front()" method.
Somethig related, zip(a,b) allows to sort the two arrys, but you can't modify the arry items with a ref foreach:
import std.stdio: writeln;
import std.algorithm: sort;
import std.range: zip;
void foo1() {
auto a = [10, 20, 30];
auto b = [100, 200, 300];
foreach (ref x, y; zip(a, b))
x++;
writeln(a);
}
void foo2() {
int[] a = [1, 2, 3];
string[] b = ["c", "b", "a"];
writeln(zip(a, b)[2]);
writeln(a, "\n", b);
sort!("a[0] > b[0]")(zip(a, b));
writeln(a, "\n", b);
}
void main() {
foo1();
foo2();
}
Output:
[10, 20, 30]
Tuple!(int,string)(3, "a")
[1, 2, 3]
["c", "b", "a"]
[3, 2, 1]
["a", "b", "c"]
Bye,
bearophile
|
July 09, 2012 Re: foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
Posted in reply to monarch_dodra | On Monday, 2 July 2012 at 12:44:59 UTC, monarch_dodra wrote:
> ...
I opened this thread a week ago, and got little no feed back.
I've realized since:
1) "map" was a bad example, since it actually returns a new range (does not modify the underlying values), so the output was normal
2) I pasted the wrong code, I forgot to put a "ref" in there, so the output was normal.
Now take this example:
----
import std.stdio;
import std.algorithm;
import std.container;
void main()
{
Array!int arr = Array!int(1, 2, 3, 4, 5);
foreach(ref a; arr[])
{
a = 10;
}
writeln("Output: ", arr[]);
}
----
Output: [1, 2, 3, 4, 5]
----
I realize I'm still new to D, but I'd appreciate if somebody told me if I am wrong(or being stupid).
Anyways, I'm bothered by several things:
Quoteth "The D programing language", 12.9.1: "If you specify ref with value, the compiler replaces all uses of value with calls to __c.front throughout the body of the loop".
Surely, there is something wrong here... right?
|
July 10, 2012 Re: foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
Posted in reply to monarch_dodra | On Monday, 9 July 2012 at 09:24:29 UTC, monarch_dodra wrote:
> ...
Forgive me for insisting, but at this point, I'm just trying to figure out if I am being stupid and misunderstood how D works.
Could someone please just tell me if what I'm seeing is expected?
|
July 10, 2012 Re: foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
Posted in reply to monarch_dodra | "monarch_dodra" <monarch_dodra@gmail.com> wrote in message news:fbithbggnynmazrxkfjw@forum.dlang.org... > On Monday, 9 July 2012 at 09:24:29 UTC, monarch_dodra wrote: >> ... > > Forgive me for insisting, but at this point, I'm just trying to figure out if I am being stupid and misunderstood how D works. > > Could someone please just tell me if what I'm seeing is expected? > > Looks like a bug. |
July 10, 2012 Re: foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
Posted in reply to monarch_dodra | On Monday, 9 July 2012 at 09:24:29 UTC, monarch_dodra wrote:
> ...
Forgive me for insisting, but at this point, I'm just trying to figure out if I am being stupid and misunderstood how D works.
Could someone please tell me if what I'm seeing is expected?
|
July 10, 2012 Re: foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
Posted in reply to monarch_dodra | There are two problems:
1. std.container.Array.(front, back, opIndex, ...) doesn't return its
elements by ref.
2. std.algorithm.map doesn't consider original range's ref-ness.
Bye.
Kenji Hara
2012/7/2 monarch_dodra <monarch_dodra@gmail.com>:
> I think this is a pretty serious bug: when one writes: "foreach(ref a,
> range)", the underlying (ref'd) object will ONLY get modified if the range
> object provides a "ref T front()" method.
>
> What is worrisome is that:
> a) The code compiles anyways.
> b) This even happens when range provides a "void front(T t)" method.
>
> Here is the bug in action, with the very standard Array class, as well as the generic Map algorithm:
> ----
> import std.container;
> import std.stdio;
> import std.algorithm;
>
> void main()
> {
> Array!int arr;
> arr.length = 3;
>
> foreach(a; arr)
> a = 2;
>
> map!("a+=2")(arr[]);
>
> writeln(arr[]);
> }
> ----
> Output:
> ----
> [0, 0, 0]
> ----
>
> As you can see, not only is "foreach" broken, but so is any algorithm that tries to mutate an Array.
>
> Note that "Array" itself can be fixed by my recommendation here: http://forum.dlang.org/thread/bkozswmsgeibarowfwvq@forum.dlang.org
>
> However, in the case of containers that can't return a ref'ed front (eg Array!bool), this is actually a double bug:
>
> 1) foreach: It makes compile time call to "T front()" regardless of context.
> Because of this, even when we write "a = 2", a straight up assignment to a
> temporary takes place, rather than compiling as "arr.front(2)".
>
> 2) front: Unlike opIndex, front is not an operator. This means that writing code such as "arr.front += 5" or "++arr.front" is not supported by the language (when front returns by value). This means the implementer of a Range (which can't return a ref'd front) has absolutely no way of intercepting operations that want to occur directly on front
|
July 10, 2012 Re: foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
Posted in reply to monarch_dodra | On 07/09/2012 02:24 AM, monarch_dodra wrote: > On Monday, 2 July 2012 at 12:44:59 UTC, monarch_dodra wrote: >> ... > > I opened this thread a week ago, and got little no feed back. Please also consider the D.learn newsgroup. Some people are more responsive over there. :) (And you may have already asked there anyway. :)) > import std.stdio; > import std.algorithm; > import std.container; > > void main() > { > Array!int arr = Array!int(1, 2, 3, 4, 5); > foreach(ref a; arr[]) > { > a = 10; > } > writeln("Output: ", arr[]); > } > ---- > Output: [1, 2, 3, 4, 5] > ---- Yet using the InputRange functions directly works: Array!int arr = Array!int(1, 2, 3, 4, 5); auto all = arr[]; for ( ; !all.empty; all.popFront()) { all.front = 10; } Now, the output: [10, 10, 10, 10, 10] Using for and indexes also works: Array!int arr = Array!int(1, 2, 3, 4, 5); auto all = arr[]; for (auto i = 0; i != all.length; ++i) { all[i] = 10; } Again, the output: [10, 10, 10, 10, 10] > I realize I'm still new to D, but I'd appreciate if somebody told me if > I am wrong(or being stupid). I have found std.container to be confusing. > Anyways, I'm bothered by several things: > > Quoteth "The D programing language", 12.9.1: "If you specify ref with > value, the compiler replaces all uses of value with calls to __c.front > throughout the body of the loop". > > Surely, there is something wrong here... right? Yes. :) Ali |
July 10, 2012 Re: foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
Posted in reply to kenji hara | On 07/10/2012 12:21 AM, kenji hara wrote: > There are two problems: > > 1. std.container.Array.(front, back, opIndex, ...) doesn't return its > elements by ref. It's not that obvious to me. The following picture should supposedly provide references to elements: The latter of the following pair of Array.Range.front() should be used in the foreach body: @property T front() { enforce(!empty); return _outer[_a]; } @property void front(T value) { enforce(!empty); _outer[_a] = move(value); } There, _outer is the Array itself. And Array.opIndexAssign() seems to be doing the right thing: void opIndexAssign(T value, size_t i) { enforce(_data.RefCounted.isInitialized); _data._payload[i] = value; } Ali |
July 10, 2012 Re: foreach ref very broken: fails to call front(val) | ||||
---|---|---|---|---|
| ||||
Posted a pull request: https://github.com/D-Programming-Language/phobos/pull/678 With the patch, following code works as expected. import std.container; import std.stdio; import std.algorithm; void main() { Array!int arr; arr.length = 3; foreach(ref a; arr) // requre 'ref' a = 2; writeln(arr[]); // prints [2,2,2] // Use writeln to evaluate all elements of map range. map!("a+=2")(arr[]).writeln(); // prints [4,4,4] } 2012/7/10 kenji hara <k.hara.pg@gmail.com>: > There are two problems: > > 1. std.container.Array.(front, back, opIndex, ...) doesn't return its > elements by ref. > 2. std.algorithm.map doesn't consider original range's ref-ness. > > Bye. > > Kenji Hara > > 2012/7/2 monarch_dodra <monarch_dodra@gmail.com>: >> I think this is a pretty serious bug: when one writes: "foreach(ref a, >> range)", the underlying (ref'd) object will ONLY get modified if the range >> object provides a "ref T front()" method. >> >> What is worrisome is that: >> a) The code compiles anyways. >> b) This even happens when range provides a "void front(T t)" method. >> >> Here is the bug in action, with the very standard Array class, as well as the generic Map algorithm: >> ---- >> import std.container; >> import std.stdio; >> import std.algorithm; >> >> void main() >> { >> Array!int arr; >> arr.length = 3; >> >> foreach(a; arr) >> a = 2; >> >> map!("a+=2")(arr[]); >> >> writeln(arr[]); >> } >> ---- >> Output: >> ---- >> [0, 0, 0] >> ---- >> >> As you can see, not only is "foreach" broken, but so is any algorithm that tries to mutate an Array. >> >> Note that "Array" itself can be fixed by my recommendation here: http://forum.dlang.org/thread/bkozswmsgeibarowfwvq@forum.dlang.org >> >> However, in the case of containers that can't return a ref'ed front (eg Array!bool), this is actually a double bug: >> >> 1) foreach: It makes compile time call to "T front()" regardless of context. >> Because of this, even when we write "a = 2", a straight up assignment to a >> temporary takes place, rather than compiling as "arr.front(2)". >> >> 2) front: Unlike opIndex, front is not an operator. This means that writing code such as "arr.front += 5" or "++arr.front" is not supported by the language (when front returns by value). This means the implementer of a Range (which can't return a ref'd front) has absolutely no way of intercepting operations that want to occur directly on front |
Copyright © 1999-2021 by the D Language Foundation