• Chris (9/9) Nov 16 2015 Updating my code from 2.067.1 to 2.069.1 (I skipped 2.068,
• Marc =?UTF-8?B?U2Now7x0eg==?= (2/11) Nov 16 2015 Well, what does `doSomething` return?
• Chris (3/19) Nov 16 2015 It returns a range that modifies individual items in myArray,
• opla (7/27) Nov 16 2015 have you...
• Chris (5/33) Nov 16 2015 I think ref is necessary, else the items are not changed. I will
• Chris (7/7) Nov 17 2015 I've checked several options now and it doesn't work.
• Marc =?UTF-8?B?U2Now7x0eg==?= (9/45) Nov 17 2015 That really depends on the details, that's why I asked. It could
• Chris (57/65) Nov 17 2015 I did just that and I could find the culprit. It's opIndex(). It
• Chris (7/7) Nov 17 2015 On Tuesday, 17 November 2015 at 11:58:22 UTC, Chris wrote:
• Marc =?UTF-8?B?U2Now7x0eg==?= (18/27) Nov 17 2015 Ok, that's a strange implementation of opIndex(). Usually, a
• Chris (3/20) Nov 17 2015 I see. Thanks for the explanation. What would be the easiest fix
• Marc =?UTF-8?B?U2Now7x0eg==?= (2/7) Nov 17 2015 Do you actually need the opIndex()? If not, just remove it.
• Chris (3/12) Nov 17 2015 I suppose that would be the easiest solution. I think I doesn't

Chris <wendlec tcd.ie> writes:

Updating my code from 2.067.1 to 2.069.1 (I skipped 2.068, 
because I was too busy).

I get this error:

invalid foreach aggregate, define opApply(), range primitives, or 
use .tupleof

for code like

foreach (ref it; myArray.doSomething) {}

Probably not the best idea anyway. What's the best fix for this? 
Thanks.

Marc =?UTF-8?B?U2Now7x0eg==?= <schuetzm gmx.net> writes:

On Monday, 16 November 2015 at 16:44:27 UTC, Chris wrote:
 Updating my code from 2.067.1 to 2.069.1 (I skipped 2.068, 
 because I was too busy).

 I get this error:

 invalid foreach aggregate, define opApply(), range primitives, 
 or use .tupleof

 for code like

 foreach (ref it; myArray.doSomething) {}

 Probably not the best idea anyway. What's the best fix for 
 this? Thanks.

Well, what does `doSomething` return?

Chris <wendlec tcd.ie> writes:

On Monday, 16 November 2015 at 16:49:19 UTC, Marc Schütz wrote:
 On Monday, 16 November 2015 at 16:44:27 UTC, Chris wrote:
 Updating my code from 2.067.1 to 2.069.1 (I skipped 2.068, 
 because I was too busy).

 I get this error:

 invalid foreach aggregate, define opApply(), range primitives, 
 or use .tupleof

 for code like

 foreach (ref it; myArray.doSomething) {}

 Probably not the best idea anyway. What's the best fix for 
 this? Thanks.

 Well, what does `doSomething` return?

It returns a range that modifies individual items in myArray, 
i.e. it assigns values to fields in each item of the array.

opla <opla opla.la> writes:

On Monday, 16 November 2015 at 16:55:29 UTC, Chris wrote:
 On Monday, 16 November 2015 at 16:49:19 UTC, Marc Schütz wrote:
 On Monday, 16 November 2015 at 16:44:27 UTC, Chris wrote:
 Updating my code from 2.067.1 to 2.069.1 (I skipped 2.068, 
 because I was too busy).

 I get this error:

 invalid foreach aggregate, define opApply(), range 
 primitives, or use .tupleof

 for code like

 foreach (ref it; myArray.doSomething) {}

 Probably not the best idea anyway. What's the best fix for 
 this? Thanks.

 Well, what does `doSomething` return?

 It returns a range that modifies individual items in myArray, 
 i.e. it assigns values to fields in each item of the array.

have you...

tried without ref ?
tried by adding a pair of parens after doSomething ?
tried std.algorithm.each or map on doSomething ?
checked the primitives ?
checked that isInputRange!(ReturnType!doSomething) == true?

Chris <wendlec tcd.ie> writes:

On Monday, 16 November 2015 at 17:57:53 UTC, opla wrote:
 On Monday, 16 November 2015 at 16:55:29 UTC, Chris wrote:
 On Monday, 16 November 2015 at 16:49:19 UTC, Marc Schütz wrote:
 On Monday, 16 November 2015 at 16:44:27 UTC, Chris wrote:
 Updating my code from 2.067.1 to 2.069.1 (I skipped 2.068, 
 because I was too busy).

 I get this error:

 invalid foreach aggregate, define opApply(), range 
 primitives, or use .tupleof

 for code like

 foreach (ref it; myArray.doSomething) {}

 Probably not the best idea anyway. What's the best fix for 
 this? Thanks.

 Well, what does `doSomething` return?

 It returns a range that modifies individual items in myArray, 
 i.e. it assigns values to fields in each item of the array.

 have you...

 tried without ref
 tried by adding a pair of parens after doSomething ?
 tried std.algorithm.each or map on doSomething ?
 checked the primitives ?
 checked that isInputRange!(ReturnType!doSomething) == true?

I think ref is necessary, else the items are not changed. I will 
try the other options tomorrow (Tuesday). Thanks.

I wonder was the change overdue (and I got away with it till 
2.068.1) or is it a new policy due to changes in D?

Chris <wendlec tcd.ie> writes:

I've checked several options now and it doesn't work.

Here (http://dlang.org/statement.html#foreach-with-ranges) it is 
stated that it suffices to have range primitives, if opApply 
doesn't exist. My code worked up to 2.068.0, with the 
introduction of 2.068.1 it failed. I wonder why that is.

I have empty, front and popFront in my range which is a struct, 
and it used to work.

Marc =?UTF-8?B?U2Now7x0eg==?= <schuetzm gmx.net> writes:

On Monday, 16 November 2015 at 18:18:51 UTC, Chris wrote:
 On Monday, 16 November 2015 at 17:57:53 UTC, opla wrote:
 On Monday, 16 November 2015 at 16:55:29 UTC, Chris wrote:
 On Monday, 16 November 2015 at 16:49:19 UTC, Marc Schütz 
 wrote:
 On Monday, 16 November 2015 at 16:44:27 UTC, Chris wrote:
 Updating my code from 2.067.1 to 2.069.1 (I skipped 2.068, 
 because I was too busy).

 I get this error:

 invalid foreach aggregate, define opApply(), range 
 primitives, or use .tupleof

 for code like

 foreach (ref it; myArray.doSomething) {}

 Probably not the best idea anyway. What's the best fix for 
 this? Thanks.

 Well, what does `doSomething` return?

 It returns a range that modifies individual items in myArray, 
 i.e. it assigns values to fields in each item of the array.

 have you...

 tried without ref
 tried by adding a pair of parens after doSomething ?
 tried std.algorithm.each or map on doSomething ?
 checked the primitives ?
 checked that isInputRange!(ReturnType!doSomething) == true?

 I think ref is necessary, else the items are not changed. I 
 will try the other options tomorrow (Tuesday). Thanks.

 I wonder was the change overdue (and I got away with it till 
 2.068.1) or is it a new policy due to changes in D?

That really depends on the details, that's why I asked. It could 
be a regression, or it could be that the compiler now does 
stricter checking than before, and your implementation wasn't 
completely correct, or it could be a bug in Phobos if you use 
that to create the range.

If you can post a minimal example that works in 2.067.1, but 
doesn't with the current version, I can try to find the change 
that broke it.

Chris <wendlec tcd.ie> writes:

On Tuesday, 17 November 2015 at 11:26:19 UTC, Marc Schütz wrote:

 That really depends on the details, that's why I asked. It 
 could be a regression, or it could be that the compiler now 
 does stricter checking than before, and your implementation 
 wasn't completely correct, or it could be a bug in Phobos if 
 you use that to create the range.

 If you can post a minimal example that works in 2.067.1, but 
 doesn't with the current version, I can try to find the change 
 that broke it.

I did just that and I could find the culprit. It's opIndex(). It 
works up until 2.068.0, with 2.068.1 I already get this error:

"primitives.d(7): Error: invalid foreach aggregate 
doSomething(items).opIndex()"

Here's the example:

=========================

import std.stdio : writeln;
import std.range.primitives;

void main()
{
   string[] items = ["a", "b", "c"];
   foreach (ref it; items.doSomething())
   {
     writeln(it);
   }
}

auto doSomething(InputRange)(ref InputRange r)
{
   struct DoSomething
   {
     private
     {
       InputRange r;
       size_t cnt;
     }

     this(InputRange r)
     {
       this.r = r;
     }

      property bool empty()
     {
       return r.length == 0;
     }

      property auto front()
     {
       return r[0];
     }

      property void popFront()
     {
       r = r[1..$];
     }

      property size_t length() const
     {
       return r.length;
     }

      property size_t opIndex()
     {
       return cnt;
     }

      property auto save() const
     {
	return this;
     }
   }

   return DoSomething(r);
}

Chris <wendlec tcd.ie> writes:

On Tuesday, 17 November 2015 at 11:58:22 UTC, Chris wrote:


Sorry that should be:

 property void popFront()
{
   r = r[1..$];
   cnt++;
}

Marc =?UTF-8?B?U2Now7x0eg==?= <schuetzm gmx.net> writes:

On Tuesday, 17 November 2015 at 11:58:22 UTC, Chris wrote:
 I did just that and I could find the culprit. It's opIndex(). 
 It works up until 2.068.0, with 2.068.1 I already get this 
 error:

 "primitives.d(7): Error: invalid foreach aggregate 
 doSomething(items).opIndex()"

      property size_t opIndex()
     {
       return cnt;
     }

Ok, that's a strange implementation of opIndex(). Usually, a 
parameter-less opIndex() is supposed to return a slice into the 
full range, but yours returns a size_t, which of course can't be 
iterated over.

The change that made this stop working is:
https://github.com/D-Programming-Language/dmd/pull/4948

This contains, among others a fix for issue 14625 "opIndex() 
doesn't work on foreach container iteration":
https://issues.dlang.org/show_bug.cgi?id=14625

This allows to iterate directly over containers, without needing 
to slice them first. I guess it's a bit too eager, because if the 
object is already iterable without slicing (as in your example), 
it could just do that. On the other hand, it's a corner case, and 
it might actually be preferable to slice always, if the iterable 
supports it...

In any case, I'd suggest you fix your opIndex(), except if 
there's a really good reason it is as it is.

Chris <wendlec tcd.ie> writes:

On Tuesday, 17 November 2015 at 12:22:22 UTC, Marc Schütz wrote:
 Ok, that's a strange implementation of opIndex(). Usually, a 
 parameter-less opIndex() is supposed to return a slice into the 
 full range, but yours returns a size_t, which of course can't 
 be iterated over.

 The change that made this stop working is:
 https://github.com/D-Programming-Language/dmd/pull/4948

 This contains, among others a fix for issue 14625 "opIndex() 
 doesn't work on foreach container iteration":
 https://issues.dlang.org/show_bug.cgi?id=14625

 This allows to iterate directly over containers, without 
 needing to slice them first. I guess it's a bit too eager, 
 because if the object is already iterable without slicing (as 
 in your example), it could just do that. On the other hand, 
 it's a corner case, and it might actually be preferable to 
 slice always, if the iterable supports it...

 In any case, I'd suggest you fix your opIndex(), except if 
 there's a really good reason it is as it is.

I see. Thanks for the explanation. What would be the easiest fix 
for this example?

Marc =?UTF-8?B?U2Now7x0eg==?= <schuetzm gmx.net> writes:

On Tuesday, 17 November 2015 at 12:41:45 UTC, Chris wrote:
 On Tuesday, 17 November 2015 at 12:22:22 UTC, Marc Schütz wrote:
 In any case, I'd suggest you fix your opIndex(), except if 
 there's a really good reason it is as it is.

 I see. Thanks for the explanation. What would be the easiest 
 fix for this example?

Do you actually need the opIndex()? If not, just remove it.

Chris <wendlec tcd.ie> writes:

On Tuesday, 17 November 2015 at 13:49:58 UTC, Marc Schütz wrote:
 On Tuesday, 17 November 2015 at 12:41:45 UTC, Chris wrote:
 On Tuesday, 17 November 2015 at 12:22:22 UTC, Marc Schütz 
 wrote:
 In any case, I'd suggest you fix your opIndex(), except if 
 there's a really good reason it is as it is.

 I see. Thanks for the explanation. What would be the easiest 
 fix for this example?

 Do you actually need the opIndex()? If not, just remove it.

I suppose that would be the easiest solution. I think I doesn't 
really need to be as it is.