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

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

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?

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?

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

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?

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

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

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

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