On 16/04/18 01:28, Jonathan M Davis wrote:
The fact that foreach copies the range that it's given makes using ref with
anything other than arrays a bit of an iffy proposition. It will compile
regardless of whether front returns by ref or not (which arguably is a bug),
but it will only affect the original elements if copying the range doesn't
copy the elements
You keep referring to ranges that contain the data, but I am racking my
mind and failing to find examples.
You can have ranges over arrays,linked lists, trees (though you'd
probably use opApply there), and any other container, as well as
external data sources (files, sockets, pipes). In none of those cases
would your range contain actual data. I need to see the use case for
ranges *with* embedded data.
Plus, as I've said before, if ranges are expected to contain data,
copying them seems like an incredibly bad idea. The whole idea behind
casually copying the range about is that it's cheap to do.
which in the case of a basic input range rather than a
forward range
I don't see how that statement is true.
is generally true, otherwise, it would be a forward range.
A forward range is an input range.
But as it stands, using ref with foreach in generic code is not going to go
well. If anything, it's actually a code smell.
You are conflating ref with owning data, which I find unsubstantiated.
If non-copyable types were allowed, then functions that accept a range
would fail to compile on non-copyable types if they do a copy.
But the flip side is that if non-copyable would be allowed, a lot of
functions that current copy would not. There are far too many
unnecessary copies in the code that serve no purpose, but they are
invisible, so they are not fixed.
And if a templated function fails to compile if it's given a particular
argument, and that failure is not due to the template constraint, that
usually means that the template constraint is incomplete.
I do not accept this argument outright. In fact, our experience at Weka
has drove us to the exact opposite direction.
We deliberately set up functions to include invalid cases and then
either let them fail because of lacking constraints, or even set up a
static assert to fail the function generation rather than fail matching.
We found that the error messages produced by the alternative are simply
too difficult to parse out and figure what went wrong.
With that said, there is some merit to setting up the function
constraints so that it matches what the function actually need. It
allows overloading the missing combinations by another module. I don't
think it is a good argument in this particular case, however.
I think we should set up to document which phobos functions require
copying. It will flesh out bugs and unnecessary copying in phobos, as
well as force phobos developers to think about this scenario when they
write code. It shouldn't even take that much time to do this run: just
go over all UTs and add instantiation over a non-copyable type. The UTs
that fail need work.
it would mean that _every_ range-based function then has to worry about
non-copyable elements
No, just those in libraries. The same way they have to worry about big O
complexity, type safety, @nogc and a bunch of other things.
When you write a library, you have to think about your users. This
doesn't change anything about that.
So, allowing
non-copyable types complicates range-based code across the board.
Like I said above, it is more likely to find bugs than to introduce
needless complications. If you code cannot be easily implemented over
non-copyable types, exclude them. Don't shut me out of the entire game.
Not all algorithms can call front just once without
copying it
Huh? Where did that come from?
Phobos already assumes that calling front needs to be an O(1) operation.
You can call front as many times as you like. Why would you assume
otherwise?
and many ranges (e.g. map) calculate front on each call, meaning
that repeated calls to front can be more expensive than just calling it once
and copying the result.
Not by a lot, and definitely not in any way the generic code has any
right to assume. I reject this argument outright.
The result of all of this is that in generic code, you have no clue
whatsoever what the semantics are of copying a range around, because it
depends entirely on the copying semantics of whatever range that code is
currently being instantiated with. As such, generic code basically has to
assume that once you copy a range, you can't mutate the original (since it
may or may not be affected by mutating the copy and may or may not be
cleanly affected by mutating the copy),
Jonathan, I'm sorry, but what you are describing is called "a bug".
Input ranges provide no guarantees about copying the range itself. If
you do it and expect *anything* (which it seems you do), you have a bug.
If you need to copy the range itself, you absolutely need to require a
forward range.
A forward range with ref front absolutely promises that both iterations
reference the same elements.
I find myself sincerely hoping that you are making these examples up in
order to win this discussion turned into an argument for some reason,
instead of the way phobos is actually written, because if you're right
phobos is *broken*.
The good news is that if Phobos is broken, this is a great way to flush
out the places it's broken. Let's add UTs calling all Phobos functions
accepting input ranges (as opposed to forward ranges) with non-copyable
ranges (i.e. - the range itself is non-copyable) and fix the compilation
errors. This shouldn't take more than a few days to do.
Hell, let's write UTs to pass forward ranges that are non-copyable (but
return a copy via "save") and flush out those bugs as well.
The point of "development" is to make our code better, not defend the
status-quo. Especially since we've just identified a fairly simple way
to find the places that need work and easily work on them.
But of course, since _most_ ranges act just like save when they're copied,
save gets frequently forgotten, making it a bit of a thorn in our side.
Then rejoice: I just suggested a way to *easily* find and fix those places.
require that forward ranges act like value types (at
least insofar as copying them copies the iteration state and thus acts like
save).
The brackets are important. Just because I'm returning a reference to
elements does not mean I don't support the range itself restarting a scan.
So, some redesign of ranges would be great.
Yes, but please note that I don't think it is *necessary*. Yes, save is
annoying, but it *is* workable as is, and since we can harness the
compiler to locate where we're deviating from the righteous way, not
that expensively if we could just be bothered to do it.
And allowing ranges with uncopyable members would turn this potential
deadly run-time bugs into easy to find compile time errors. Sound like a
great reason to allow it, to me.
Ranges have to be copyable.
And why would a range iterating over uncopyable objects not be copyable?
How are the two related?
Shachar
