Transience of .front in input vs. forward ranges

[H. S. Teoh]

On Sun, Nov 04, 2012 at 11:58:18PM -0800, Jonathan M Davis wrote:
> On Sunday, November 04, 2012 21:07:46 H. S. Teoh wrote:
> > Stop right there. You're contradicting yourself. You kept saying
> > that transient ranges are rare, abnormal, etc., and now you say they
> > are a potentially large number of range types? I'm sorry, you've
> > managed to utterly confuse me.  Are they rare, or are they not?
> > 
> > If they are rare, then this thin wrapper only needs to be written in
> > those few rare cases. *No other range type needs to be changed.*
> > 
> > If they are common, then they aren't abnormal, and we should be
> > fixing Phobos to deal with them. Everywhere. In a pervasive,
> > invasive, large changeset, because almost all current code is broken
> > w.r.t. to these common cases -- if indeed they are common.
> 
> Base ranges with transient fronts are rare, but as soon as you have to
> take them into account with wrapper ranges, then they potentially
> affect almost every range-based function in Phobos. So, you're taking
> a very rare case and forcing _everything_ to account for it. _That_ is
> why it affects a large number of range types.

Only wrapper ranges that explicit support transience need to propagate
it. Wrapper ranges that don't support transience, by definition, should
*not* propagate it, because that would be wrong (they don't support
transience because they don't handle transient .front correctly, so they
should not propagate it at all, otherwise they will exhibit buggy
behaviour).


[...]
> > As I said, by having a UFCS version of .transient that simply
> > returns the original range, you don't even need to know what
> > .transient does.  Don't even include it in your range, and it just
> > behaves as a normal non-transient range. How is that any more
> > complicated?
> 
> You can't just return the original range from a wrapper range and have
> it work. Take filter for instance, if you got at the range that it
> wrapped, you'd be completely bypassing filter, and the range wouldn't
> be filtered at all.

No, you're misunderstanding how it works. Say I have this range:

	struct MyRange {
		@property auto front() { ... }
		@property bool empty() { ... }
		void popFront() { ... }
		auto transient() { return MyTransientRange(this); }
	}

Now, say filter doesn't support transience. In which case we simply
do this:

	auto filter(alias pred, R)(R range) {
		struct Filter(alias pred) {
			@property auto front() { /* filtered front */ }
			@property bool empty() { ... }
			void popFront() { ... }
			// NB: No .transient is defined
		}
		return Filter!pred(range);
	}

Now we call filter on our range:

	MyRange r;
	auto filtered = filter!"a<10"(r);

What happens? Filter doesn't define .transient, so UFCS kicks in with
the default function that simply returns Filter!pred (NOT r). This is
the correct behaviour, because filter() doesn't support transience. It
would be wrong to add this line to struct Filter:

	auto transient() { return _filterImpl(range.transient); }

because the filter implementation does NOT support transience. So in
this case, .transient shouldn't be propagated.


> Any and all wrapper ranges would have to take transient into account.
> If they couldn't do transient ranges, then they just wouldn't
> propagate or use transient. But if they could, then they'd have to
> create their own transient property which returned an appropriate
> range type which functioned properly with transience. Yes, it would
> use the transient property of the range that it was wrapping, but it
> would have to create its own transient property which functioned
> properly with whatever it was doing.

Which is only a small cost, as I demonstrated with the thin wrapper.


> The _only_ way that this doesn't affect a lot of ranges in Phobos is
> if we just don't bother to propagate the transient property, and if we
> don't bother to propagate it, then there was no point in having it in
> the first place. Such ranges might as well have their own functions
> for iterating or just use opApply if no wrapper ranges are going to
> propagate their extra properties or if no range-based functions take
> those extra properties into account.
[...]

Like I said, it only affects Phobos ranges that explicitly support
transience. Those are the ranges that *should* be modified in the first
place. Those that don't support .transient, simply don't propagate it
(and they *shouldn't* propagate it, because that would be a bug).  No
changes are required in that case.

And of those ranges that should support transience, well, they are
already failing today to work with transient ranges, so they need to be
fixed anyway. With this proposal, we even get to delay the fix because
we make all ranges non-transient by default, so the broken code will
actually start working correctly, even *before* we fix it(!).


T

-- 
People tell me that I'm paranoid, but they're just out to get me.