Jump to page: 1 2 3
Thread overview
[Issue 14478] isInputRange should allow ranges of non-copyable elements
[Issue 14478] isInputRange failed to recognize some ranges
Apr 21, 2015
Ketmar Dark
Apr 21, 2015
Brian Smith
Apr 21, 2015
Jonathan M Davis
Apr 21, 2015
Ketmar Dark
Apr 22, 2015
Jonathan M Davis
Apr 22, 2015
Peter Alexander
Apr 22, 2015
Jonathan M Davis
Apr 22, 2015
Peter Alexander
Apr 22, 2015
Peter Alexander
Apr 23, 2015
Jonathan M Davis
Apr 23, 2015
Martin Nowak
Apr 23, 2015
Jonathan M Davis
Mar 04, 2016
greenify
Jul 21, 2017
Vladimir Panteleev
Sep 19, 2019
Dlang Bot
Oct 25, 2019
Jonathan M Davis
Dec 17, 2022
Iain Buclaw
Mar 12, 2023
Jan Jurzitza
Mar 16, 2023
Dlang Bot
May 01, 2023
Dlang Bot
April 21, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

--- Comment #1 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
seems that `isBidirectionalRange` can be affected too.

--
April 21, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

Brian Smith <block8437@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |block8437@gmail.com

--- Comment #2 from Brian Smith <block8437@gmail.com> ---
*** Issue 14479 has been marked as a duplicate of this issue. ***

--
April 21, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

Jonathan M Davis <issues.dlang@jmdavisProg.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |issues.dlang@jmdavisProg.co
                   |                            |m

--- Comment #3 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
If

auto h = r.front;

doesn't work, then I don't think that it can be a range. And if that means that elements have to be copyable to be in a range, then I think that they're going to have to be copyable. There is going to be _way_ too much code out there that does

auto h = r.front;

for it to work to try and claim that something is an input range when that line doesn't work. And IIRC, C++ STL containers require that their elements be copyable in order to work, so it's not like we'd be the first to require this sort of thing.

--
April 21, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

--- Comment #4 from Ketmar Dark <ketmar@ketmar.no-ip.org> ---
yet at least some std.algorithm algorithms are able to work with such ranges. chain works ok, filter works too. others will do too, i think, with `(ref a)` predicates. yet current implementation forbids such usage.

--
April 22, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

--- Comment #5 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
Sure, some may work, but many won't, and a lot of user code out there won't. Grepping for such usage in Phobos quickly finds some, and a function like std.array.array _can't_ work with non-copyable elements (and that's a pretty major range-based function in Phobos).

auto h = r.front;

has been part of the definition of input ranges from the beginning, and so, everyone has been free to assume that a range's elements are copyable, because the definition of an input range guarantees it. We can't expect that it won't break code to change that now - especially when having non-copyable elements is so rare. This is the first time that I've ever heard anyone complain about this issue, so it clearly isn't affecting very many people, and the C++ folks thought that it was okay to require that elements in STL containers be copyable, so we're not the first to require that sort of thing with a major component of the standard library.

I can see why you'd want to able to have ranges work with non-copyable elements, but simply changing the definition of an input range would break too much code, and I think that it's naive to expect that most range-based code would work with the change. I've seen plenty of cases in code where something like

auto value = range.front;

is done, and if this change were made, then that code's template constraints would be claiming that it worked with non-copyable elements when it didn't, and as it is now, such template constraints actually guarantee that non-copyable elements aren't used with such algorithms.

If we were going to support anything like this, then we'd need a new set of traits for it rather than simply changing isInputRange, and given how rare this use case is, I don't think that it's worth it. I'd suggest that you just wrap a would-be range with non-copyable elements in another range which makes it so that they _are_ copyable (e.g. via pointer) and get around the problem that way.

--
April 22, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

Peter Alexander <peter.alexander.au@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |peter.alexander.au@gmail.co
                   |                            |m

--- Comment #6 from Peter Alexander <peter.alexander.au@gmail.com> ---
(In reply to Jonathan M Davis from comment #3)
> <snip> And IIRC, C++ STL containers require that their elements
> be copyable in order to work, so it's not like we'd be the first to require
> this sort of thing.

Most containers in C++ only require movability, otherwise you couldn't have containers of std::unique_ptr<T>.

--
April 22, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

--- Comment #7 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
> Most containers in C++ only require movability, otherwise you couldn't have containers of std::unique_ptr<T>

Then that's a change with C++11.

Regardless, while it might be nice to support non-copyable elements with ranges, I really don't think that it's worth it - particularly when isInputRange has always required copyability. We'd be forced to either create a separate set of basic range traits - e.g. isInputRangeWithNonCopyableElements - or add something like hasCopyableElements and change isInputRange to no longer require copyable elements, which would then mean that a large portion of the range-based code out there would be wrong, since a lot of it is going to be copying elements, and unless folks regularly test their range-based code with ranges with non-copyable elements, they're not likely to catch the bugs where they accidentally require it, and they could be in for some rude surprises later when someone tries to use non-copyable elements. We already have enough problems with code assuming forward ranges or assuming that ranges aren't full reference types, which cause all kinds of fun bugs - including in Phobos.

--
April 22, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

--- Comment #8 from Peter Alexander <peter.alexander.au@gmail.com> ---
(In reply to Jonathan M Davis from comment #7)
> > Most containers in C++ only require movability, otherwise you couldn't have containers of std::unique_ptr<T>
> 
> Then that's a change with C++11.

Correct. Move semantics didn't really exist before C++11.


> Regardless, while it might be nice to support non-copyable elements with ranges, I really don't think that it's worth it - particularly when isInputRange has always required copyability. We'd be forced to either create a separate set of basic range traits - e.g. isInputRangeWithNonCopyableElements - or add something like hasCopyableElements and change isInputRange to no longer require copyable elements, which would then mean that a large portion of the range-based code out there would be wrong, since a lot of it is going to be copying elements, and unless folks regularly test their range-based code with ranges with non-copyable elements, they're not likely to catch the bugs where they accidentally require it, and they could be in for some rude surprises later when someone tries to use non-copyable elements. We already have enough problems with code assuming forward ranges or assuming that ranges aren't full reference types, which cause all kinds of fun bugs - including in Phobos.

I'd propose your second options: we already have hasMobileElements, hasAssignableElements etc., so I see no harm adding hasCopyableElements, and relax the isInputRange restrictions.

Worst case scenario: someone has a range algorithm that expects copyable elements, but doesn't test for it, and tries to use it with move-only elements. In this case, they'll get an error on the line that tries to do the copy.

Other than that, I don't believe any existing code will break.

--
April 22, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

--- Comment #9 from Peter Alexander <peter.alexander.au@gmail.com> ---
Also, I just want to say that I think this is important. The only reason we aren't seeing its importance right now is because we have few non-copyable, but moveable types. Barely anyone uses Unique. Once Unique becomes more popular, I don't think we can justify a world where it's not possible to have an input range over a container of Unique objects.

--
April 23, 2015
https://issues.dlang.org/show_bug.cgi?id=14478

--- Comment #10 from Jonathan M Davis <issues.dlang@jmdavisProg.com> ---
Well, having something like Unique in the standard library not work with input ranges does sound worse, though I've rarely had a need for that sort of thing, and I'm quite certain that a lot of range-based code copies elements right now without considering the possibility of elements that aren't copyable.

Even if we _do_ want to make it so that input ranges work with non-copyable types though, does it make any sense to do so with forward ranges - particularly when you consider having movable elements? Though that raises the question as to how well we do with movable elements and forward ranges right now. I don't think that most ranges have movable elements, but what happens when a range has been saved and then an element from that range is moved? Does the saved range still have a copy of it? Since it's supposed to be a copy of the range, ideally, it would, but realistically - particularly from the standpoint of a range being a view into a container - I don't see how it could. So, I suspect that we're really not dealing with the movability of elements cleanly right now either.

Regardless, I really think that this is the sort of decision that Andrei needs to be involved in, and I have no idea what his take on it will be. Allowing non-copyable elements definitely makes things more complicated - particularly when it involves adding yet another range trait (and we arguably have too many already) - but I could also see it looking bad if Unique can't work in ranges, particularly if the STL now works with movable elements and not just copyable ones.

This is just ugly all around.

--
« First   ‹ Prev
1 2 3