Fix 12564, 14493: add std.range.walkBack to get the last element of an input range

[wilzbach]

This seems to be a missing functionality (when I bumped into this I found two open issues). It behaves as walkLength.

It got a bit messy because Rebindable isn't a thing yet (see #4363 for more details).

[dlang-bot]

Fix	Bugzilla	Description
✓	12564	std.range.backWalk, std.range.backWalkMaybe
✓	14493	std.range.walkBack too

[pineapplemachine]

It would be good to provide another overload which accepts a fallback value returned when the input is empty rather than causing an error

[wilzbach]

There are at least these options:

• use Nullable
• provide an (.ifEmpty(value)) method that will yield a range of length 1 with the default value if an empty range is passed in and the otherwise the range unmodified
• use enforce instead (generally disliked, because exceptions allocate)
• provide an overload with a seed

The best precedent I found is maxElement, so I guess we should follow this then.

[wilzbach]

See #5154 for further discussion about handling empty ranges.

[andralex]

orElse yum

[pineapplemachine]

This can be done without Unqual or Rebindable.

while(!range.empty){
    auto element = range.front;
    range.popFront();
    if(range.empty) return element;
}

[wilzbach]

Yeah, but unfortunately that is slower:

> ldc -O5 -release -boundscheck=off walkBack.d && ./walkBack
walkBack.foreach = 3 secs, 772 ms, 463 μs, and 6 hnsecs
walkBack.while  = 4 secs, 191 ms, 417 μs, and 8 hnsecs

benchmark code

[pineapplemachine]

I'm having issues testing using your code with DMD. As it was, the first while test was consuming the range and subsequent tests failed because the range had been consumed.

I fixed it by preceding each call with auto r = range; and operating on r instead, but this served only to make the while approach dramatically slower while not significantly affecting the performance of the foreach test.

I would've expected that rewriting like so might make their performance more similar, but I'm surprised the compiler wasn't able to optimize the structure to look more like this anyway:

bool empty = range.empty;
while(!empty){
    auto element = range.front;
    range.popFront();
    empty = range.empty;
    if(empty) return element;
}

@UplinkCoder can you explain what's going on here?

[UplinkCoder]

the foreachloop should be rewritten to this:

{
  typeof(range.front()) elm = range.front;
  while(!range.empty)
    {
      ForeachBody()
      range.popFront();
      elm = range.front;
    }
}

[DmitryOlshansky]

Rather this:

{
 
  while(!range.empty)
    {
      typeof(range.front()) elm = range.front;
      ForeachBody()
      range.popFront();
    }
}

[DmitryOlshansky]

Truth be told all of these new names to fill every minor need make my head spin. What's the semantics of something like walkBack - I can't tell at glance.

[jmdavis]

Truth be told all of these new names to fill every minor need make my head spin. What's the semantics of something like walkBack - I can't tell at glance.

Yeah, and while this probably is something that folks need occasionally, it doesn't seem like all that great an idea in general given how horribly inefficient it is. As such, if we were to add it, I'd be tempted to argue that something descriptive and ugly like iterateAndReturnLastElement would be better.

But as the second linked bug report mentions, std.range.taillargely fixes this problem already. It just doesn't work with basic input ranges unless they havelength(withoutlength, it needs at least a foward range). So, wouldn't it make more sense to extend tail` to work with basic input ranges than to add this function?

[wilzbach]

Truth be told all of these new names to fill every minor need make my head spin. What's the semantics of something like walkBack - I can't tell at glance.

The idea behind the walk* wrapper methods is to tell by its name that it potentially has a different performance as the entire range might need to be walked.

As such, if we were to add it, I'd be tempted to argue that something descriptive and ugly like iterateAndReturnLastElement would be better.

If this functionality be added, I would argue to stay in the naming scheme as walkLength already exists. After all it's not called iterateAndReturnLength.

But as the second linked bug report mentions, std.range.taillargely fixes this problem already. It just doesn't work with basic input ranges unless they havelength(withoutlength, it needs at least a foward range). So, wouldn't it make more sense to extend tail` to work with basic input ranges than to add this function?

Two random points:

1. How would you do this within @nogc at runtime?
   AFAICT it would only work as template argument, e.g. tail!5

2. Assume we would have a.buffer method which is reasonable for allocating (and clearly marked as such), one might be able to do sth. like this without needing to touch tail:

input.buffer(5).tail(5);

[DmitryOlshansky]

The idea behind the walk* wrapper methods is to tell by its name that it potentially has a different performance as the entire range might need to be walked.

Oh, now I get it so this is like walkLength. I initially read walkBack as an imperative which sounds more like walk the range backwards. Still sucks I guess as I'd never reach out for that intuitively.

(when I bumped into this I found two open issues)

By the same person who absolutely loves filling enhancement requests, sometimes it seems to me that we often overlook an option of WONTFIX.

[andralex]

Knee-jerk reaction: how does this compare to r.tail(1)?

[wilzbach]

Knee-jerk reaction: how does this compare to r.tail(1)?

See above for a longer motivation, but in short like walkLength it works for any range (tail doesn't).

[andralex]

Understood. So we have a range that is input but not forward, and want to advance it through the end to get the last element. What's wrong with auto last = r.fold!((a, b) => b);?

[wilzbach]

What's wrong with auto last = r.fold!((a, b) => b);?

Nothing - I am closing this and the existing bugs I found as WONTFIX
However at least did this PR spark the discussion about range APIs for empty range (Nullable, orElse, ...)

[andralex]

@wilzbach yes awesome and actually that's a rather clever (ahem) use of fold. I felt pretty good about myself :). Best outcome of this: a ddoc unittest of fold that prints the last line of a file.

[wilzbach]

Best outcome of this: a ddoc unittest of fold that prints the last line of a file.

Sure -> #5227