Jump to page: 1 2
Thread overview
Unexpected foreach lowering
Aug 10, 2016
Lodovico Giaretta
Aug 10, 2016
Lodovico Giaretta
Aug 10, 2016
Ali Çehreli
Aug 10, 2016
Lodovico Giaretta
Aug 10, 2016
Ali Çehreli
Aug 10, 2016
Lodovico Giaretta
Aug 10, 2016
Lodovico Giaretta
Aug 10, 2016
Lodovico Giaretta
Aug 11, 2016
Jonathan M Davis
Aug 11, 2016
Jonathan M Davis
Aug 10, 2016
ag0aep6g
August 10, 2016
I'm probably missing something stupid but...
Why on earth do the two loops in main print a different result?
It looks like the foreach lowering is ignoring my definition of front...

=====================================================
import std.stdio, std.container.array;

struct RangeWrapper(Range)
{
	Range range;
	alias range this;
	
	auto front()
	{
		return range.front + 1;
	}
}
auto rangeWrapper(Range)(auto ref Range range)
{
	return RangeWrapper!Range(range);
}

void main()
{
	Array!int array;
	array.insertBack(3);
	
	foreach (i; rangeWrapper(array[]))
		writeln(i);           // prints 3, which is wrong
	
        // isn't the above foreach equivalent to the following loop ?

	for (auto r = rangeWrapper(array[]); !r.empty; r.popFront())
		writeln(r.front);     // correctly prints 4
}
=====================================================

Thank you for your help.
August 10, 2016
On Wednesday, 10 August 2016 at 18:08:02 UTC, Lodovico Giaretta wrote:
> I'm probably missing something stupid but...
> Why on earth do the two loops in main print a different result?
> It looks like the foreach lowering is ignoring my definition of front...
>
> =====================================================
> import std.stdio, std.container.array;
>
> struct RangeWrapper(Range)
> {
> 	Range range;
> 	alias range this;
> 	
> 	auto front()
> 	{
> 		return range.front + 1;
> 	}
> }
> auto rangeWrapper(Range)(auto ref Range range)
> {
> 	return RangeWrapper!Range(range);
> }
>
> void main()
> {
> 	Array!int array;
> 	array.insertBack(3);
> 	
> 	foreach (i; rangeWrapper(array[]))
> 		writeln(i);           // prints 3, which is wrong
> 	
>         // isn't the above foreach equivalent to the following loop ?
>
> 	for (auto r = rangeWrapper(array[]); !r.empty; r.popFront())
> 		writeln(r.front);     // correctly prints 4
> }
> =====================================================
>
> Thank you for your help.

This actually only happens with std.container.Array. Other ranges are ok. Even stranger...
August 10, 2016
On 08/10/2016 11:08 AM, Lodovico Giaretta wrote:
> I'm probably missing something stupid but...
> Why on earth do the two loops in main print a different result?
> It looks like the foreach lowering is ignoring my definition of front...
>
> =====================================================
> import std.stdio, std.container.array;
>
> struct RangeWrapper(Range)
> {
>     Range range;
>     alias range this;
>
>     auto front()
>     {
>         return range.front + 1;
>     }
> }
> auto rangeWrapper(Range)(auto ref Range range)
> {
>     return RangeWrapper!Range(range);
> }
>
> void main()
> {
>     Array!int array;
>     array.insertBack(3);
>
>     foreach (i; rangeWrapper(array[]))
>         writeln(i);           // prints 3, which is wrong
>
>         // isn't the above foreach equivalent to the following loop ?
>
>     for (auto r = rangeWrapper(array[]); !r.empty; r.popFront())
>         writeln(r.front);     // correctly prints 4
> }
> =====================================================
>
> Thank you for your help.

RangeWrapper does not provide the InputRange interface, so the compiler uses 'alias this' and iterates directly on the member range.

I tried making RangeWrapper an InputRange but failed. It still uses 'range'.

// Still fails with these:
    @property bool empty() {
        return range.empty;
    }

    void popFront() {
        range.popFront();
    }

I don't know how the decision process works there.

Ali

August 10, 2016
On Wednesday, 10 August 2016 at 18:38:00 UTC, Ali Çehreli wrote:
> RangeWrapper does not provide the InputRange interface, so the compiler uses 'alias this' and iterates directly on the member range.
>
> I tried making RangeWrapper an InputRange but failed. It still uses 'range'.
>
> // Still fails with these:
>     @property bool empty() {
>         return range.empty;
>     }
>
>     void popFront() {
>         range.popFront();
>     }
>
> I don't know how the decision process works there.
>
> Ali

That's strange, as RangeWrapper works correctly if instantiated with any underlying range EXCEPT std.container.Array.
Also, RangeWrapper does provide the InputRange interface, partially directly and partially with alias this. RangeWrapper should be "opaque", as it should not matter whether the methods needed for the InputRange interface are defined directly or inherited with alias this.
August 10, 2016
On 08/10/2016 11:47 AM, Lodovico Giaretta wrote:
> On Wednesday, 10 August 2016 at 18:38:00 UTC, Ali Çehreli wrote:
>> RangeWrapper does not provide the InputRange interface, so the
>> compiler uses 'alias this' and iterates directly on the member range.
>>
>> I tried making RangeWrapper an InputRange but failed. It still uses
>> 'range'.
>>
>> // Still fails with these:
>>     @property bool empty() {
>>         return range.empty;
>>     }
>>
>>     void popFront() {
>>         range.popFront();
>>     }
>>
>> I don't know how the decision process works there.
>>
>> Ali
>
> That's strange, as RangeWrapper works correctly if instantiated with any
> underlying range EXCEPT std.container.Array.

A quick read reveals popFront() is implemented only for bool Arrays. That explains the issue.

I don't know whether it's an oversight.

Ali

August 10, 2016
On Wednesday, 10 August 2016 at 19:37:39 UTC, Ali Çehreli wrote:
> A quick read reveals popFront() is implemented only for bool Arrays. That explains the issue.
>
> I don't know whether it's an oversight.
>
> Ali

First of all, thank you for spending your time on this issue. I really appreciate that.

I may be reading that code wrong but...
Isn't popFront implemented for every type here?
https://github.com/dlang/phobos/blob/master/std/container/array.d#L128
August 10, 2016
On 8/10/16 2:08 PM, Lodovico Giaretta wrote:
> I'm probably missing something stupid but...
> Why on earth do the two loops in main print a different result?
> It looks like the foreach lowering is ignoring my definition of front...
>
> =====================================================
> import std.stdio, std.container.array;
>
> struct RangeWrapper(Range)
> {
>     Range range;
>     alias range this;
>
>     auto front()
>     {
>         return range.front + 1;
>     }
> }
> auto rangeWrapper(Range)(auto ref Range range)
> {
>     return RangeWrapper!Range(range);
> }
>
> void main()
> {
>     Array!int array;
>     array.insertBack(3);
>
>     foreach (i; rangeWrapper(array[]))
>         writeln(i);           // prints 3, which is wrong
>
>         // isn't the above foreach equivalent to the following loop ?
>
>     for (auto r = rangeWrapper(array[]); !r.empty; r.popFront())
>         writeln(r.front);     // correctly prints 4
> }
> =====================================================
>
> Thank you for your help.

The issue is that it tries using [] on the item to see if it defines a range-like thing. Since you don't define opSlice(), it automatically goes to the subrange.

This breaks for int[] as well as Array.

If I add opSlice to your code (and return this) it works.

This is definitely a bug, it should try range functions before opSlice. Please file.

-Steve
August 10, 2016
On Wednesday, 10 August 2016 at 20:54:15 UTC, Steven Schveighoffer wrote:
> On 8/10/16 2:08 PM, Lodovico Giaretta wrote:
>> [...]
>
> The issue is that it tries using [] on the item to see if it defines a range-like thing. Since you don't define opSlice(), it automatically goes to the subrange.
>
> This breaks for int[] as well as Array.
>
> If I add opSlice to your code (and return this) it works.
>
> This is definitely a bug, it should try range functions before opSlice. Please file.
>
> -Steve

Wow. Thanks. I didn't know the compiler would try opSlice. I will file it.
August 10, 2016
On Wednesday, 10 August 2016 at 21:00:01 UTC, Lodovico Giaretta wrote:
> On Wednesday, 10 August 2016 at 20:54:15 UTC, Steven Schveighoffer wrote:
>> [...]
>
> Wow. Thanks. I didn't know the compiler would try opSlice. I will file it.

Filed on bugzilla:

https://issues.dlang.org/show_bug.cgi?id=16374
August 10, 2016
On 08/10/2016 10:54 PM, Steven Schveighoffer wrote:
> The issue is that it tries using [] on the item to see if it defines a
> range-like thing. Since you don't define opSlice(), it automatically
> goes to the subrange.
>
> This breaks for int[] as well as Array.
>
> If I add opSlice to your code (and return this) it works.
>
> This is definitely a bug, it should try range functions before opSlice.
> Please file.

Related:

https://issues.dlang.org/show_bug.cgi?id=14619
« First   ‹ Prev
1 2