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.

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?

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.

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?

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?

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.

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.

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);
}

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


Sorry that should be:

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

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.