foreach over const input range fails

[dlangBugzillaToGithub]

Alex Goltman reported this on 2017-08-21T13:00:22Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=17771

CC List

• ag0aep6g
• Eyal
• Jonathan M Davis (@jmdavis)
• Steven Schveighoffer (@schveiguy)

Description

```
    static struct NumRange {
        int front = 0;
        int end = 0;

        @property auto length() const { return end - front; }
        @property bool empty() const { return length == 0; }
        void popFront() { front++; }
    }
    const r = NumRange(0, 5);
    foreach (x; r) {
        writeln(x);
    }
```
results in
```
Error: mutable method test.main.NumRange.popFront is not callable using a const object
```
even though foreach copies the input range it gets (as evident from running foreach on same range twice), so it can at least try to copy it to an non-const.

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2017-08-21T14:48:46Z

1. foreach (and D in general) doesn't attempt to change any attributes, it simply makes a copy via:

auto _r = range;

2. Ranges aren't always copyable to non-const versions implicitly.

So you need to do this yourself or cast:

foreach(x; cast() r)

[dlangBugzillaToGithub]

alex.goltman commented on 2017-08-21T15:37:44Z

If it fails to implicitly copy to a non-const (e.g.  a struct with a pointer inside) then so be it - but why not make it try at least? as in:

Unqual!(typeof(range)) _r = range;

It seems weird that foreach which intentionally copies the range to avoid modifying it will fail because a range is an unmodifiable const.

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2017-08-21T15:47:55Z

The compiler doesn't attempt to modify attributes in almost any case.

For instance:

auto foo()
{
   const int x = 5;
   return x;
}

auto y = foo(); // y is const(int), even though it could be just int

So this behavior is consistent. You need to tell the compiler to make the copy with the correct attributes.

Note that many many ranges are not implicitly copyable to a mutable version, so this ability would not help in most cases.

One valid enhancement that you could propose is to add a function somewhere in phobos (probably in typecons) that returns a copy of the item with as many qualifiers removed as possible. Then your code could become something like:

foreach(x; unqual(r))

[dlangBugzillaToGithub]

eyal commented on 2017-08-22T06:20:01Z

Leaving the "const" there (for input ranges) will cause compile-errors (popFront generally requires mutability).

Removing the attribute will sometimes fail.
Keeping the attribute will always fail.

For the cases where it need not fail - why fail?

It may not be consistent with other behaviors - but there is no case for which it has worse behavior. It is much more useful.

D has many inconsistencies for usefulness:

* Pointers are dereferenced in some contexts but not others
* Integers have valid init values, floats have invalid ones
* Arrays and slices have specific foreach behavior and not the input-range behavior
* Tuples have special handling in the language

Shouldn't ability to iterate a const/immutable range be important enough to warrant a slight inconsistency?

foreach can simply be defined as calling unqual on its argument.

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2017-08-22T13:39:13Z

But the problem is that almost all range code is templated, so you are going to test with something that works, and then it will fail with something that doesn't. Subtle inconsistencies like this have a cost.

I'll also note that a const range, in this case, is not actually a range:

const NumRange n;

static assert(!isInputRange!(typeof(n)));

This would result in a stream of bugs like "it works with foreach but not with map, why not?"

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2017-08-22T13:44:49Z

(In reply to Eyal from comment #4)
> * Pointers are dereferenced in some contexts but not others
Not sure about this one. Perhaps an example can explain better?

> * Integers have valid init values, floats have invalid ones
And it is a cause of problems IMO, especially with generic code.

> * Arrays and slices have specific foreach behavior and not the input-range
> behavior
Again, causes lots of problems:
string s = "hello";
pragma(msg, typeof(s.front())); // immutable dchar
foreach(x; s)
   pragma(msg, typeof(x)); // immutable char

> * Tuples have special handling in the language
Since obsoleted with a better new feature (static foreach). Again, this is a source of problems, and not meant as a model to follow.

[dlangBugzillaToGithub]

alex.goltman commented on 2017-08-22T15:03:45Z

Actually map does works on const NumRange, since it's declared as:

    auto map(Range)(Range r) if (isInputRange!(Unqual!Range))

Why shouldn't foreach work as well? 

It's not a critical bug, but fixing this would remove an annoyance which would make D users much less frustrated. Please don't dismiss this before really evaluating what would it take to fix it, because currently we don't see a reason for this not to just work, as map does already.

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2017-08-22T15:32:54Z

(In reply to Alex Goltman from comment #7)
> Actually map does works on const NumRange, since it's declared as:
> 
>     auto map(Range)(Range r) if (isInputRange!(Unqual!Range))

Yep, I didn't realize that, ironic that I picked map as the example :)

However, there are other functions that aren't checked this way, and most user code isn't going to do this. It's the wrong way to approach this IMO.

> Why shouldn't foreach work as well? 

I'll leave the issue open, as there obviously are some in the core team who find such subtleties worth considering, given the state of std.algorithm. I still don't these kinds of tricks are worth the confusion they can cause, especially when a suitable converter is easily had and used.

[dlangBugzillaToGithub]

alex.goltman commented on 2017-08-22T17:10:47Z

Asking the user to use a converter makes sense when the user understands there's a reason to convert. 
Here it's counter-intuitive - the error stating foreach doesn't work with a const actually made me question whether foreach over a range modifies the range I give it, and I've somehow been unaware of this the whole time. 
The current state is more confusing IMHO.

[dlangBugzillaToGithub]

schveiguy (@schveiguy) commented on 2017-08-22T17:56:14Z

It's actually counter-intuitive for the D compiler to strip qualifiers automatically, it almost never happens.

The only place I'm aware of is IFTI for arrays and pointers.

I feel like this is a case of being helpful, but will be confusing because it's not consistent for all ranges. Experience shows that the benefit is not worth the confusion.

But I am not the decider of such things, so I will leave it up to Walter/Andrei to comment.

[dlangBugzillaToGithub]

ag0aep6g commented on 2017-08-22T19:37:02Z

(In reply to Alex Goltman from comment #9)
> the error stating foreach doesn't work with a
> const actually made me question whether foreach over a range modifies the
> range I give it,

I don't really have a horse in this race, but if we consider a generic range, then `foreach` does very much modify it. `foreach` doesn't call `save`, so every `popFront` potentially modifies the original range.

It doesn't really matter that `foreach` copies the range, because copying is not a range concept. If you have a generic range, and you run it through `foreach`, you must be prepared to have an empty range afterwards.

[dlangBugzillaToGithub]

issues.dlang (@jmdavis) commented on 2017-08-22T21:40:47Z

For const to work with ranges, we'd really need some sort of tail-const slicing mechanism similar to what happens with dynamic arrays and IFTI (though presumably, it would require help from the programmer as opposed to being automatic, since it really can't be done automatically with ranges like it can be with arrays). But wthout some form of tail-const mechanism, it's a waste of time to muck around with const and ranges. Best case, you make certain use cases work while having a bunch that don't. At this point, it's far better to just never use const with ranges. They're pretty much inherently incompatible given the fact that iterating a range modifies it, and the only defined way to copy a range is save. Technically, even using a range after passing it to a function or using foreach on it is undefined behavior in generic code, since the copy semantics differ wildly depending on how the range is defined. So, trying to do anything with const automatically is a total minefield. My answer to anyone trying to use const with ranges is to just give up now. They simply don't work together, and trying to make them work together without language additions related to tail-const is just going to cause you grief in the long run.

[dlangBugzillaToGithub]

snarwin+bugzilla commented on 2022-02-23T21:11:18Z

*** Issue 18631 has been marked as a duplicate of this issue. ***