Duncan P. N. Exon Smith
2014-Mar-19 18:57 UTC
[LLVMdev] [RFC] Add empty() method to iterator_range.
On Mar 19, 2014, at 11:32 AM, Aaron Ballman <aaron at aaronballman.com> wrote:> On Wed, Mar 19, 2014 at 2:13 PM, Lang Hames <lhames at gmail.com> wrote: >> Hi all, >> >> As RFCs go this is short and sweet - I think it would be nice to add an >> empty method to iterator_range. Something like: >> >> bool empty() const { return begin() != end(); } >> >> My motivation is to make the 'if' test at the start of this common pattern >> tidier: >> >> if (!collection.empty()) >> // initialization... >> for (auto& c : collection) >> // do something for each c. >> >> which I think that is preferable to: >> >> if (collection.begin() != collection.end()) >> // initialization... >> // for each... >> >> The empty method is just a single iterator comparison, so it should be valid >> and cheap for any underlying iterator type. >> >> Pros: >> - Enables small aesthetic improvements. >> - Would make iterator_range look slightly more "container-like", so it could >> substitute into more template code, though I'd expect templates that take >> read-only containers rather than iterators to be very rare in C++, and >> templates that take collections and use only begin, end and empty to be >> rarer still. >> >> Cons: ?I’m in favour.> This is a pattern I ran into a handful of times when range-ifying > parts of clang (but not so frequently that I felt it was a major win), > so I'm roughly in favor of the API. However, if there's been > standardization efforts for range, we should be sure that we're doing > something compatible there.There are a couple of proposals [1][2] that I can find. It sounds like the exact set of methods to provide is somewhat contentious [3]. empty() seems pretty innocuous though. Note that the standard proposals use std::range instead of std::iterator_range. In my opinion, other methods (e.g., front(), back(), and (for random access iterators) operator[]) would also be useful. [1]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1871.html [2]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3350.html [3]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3513.html#range-arg
Alexey Samsonov
2014-Mar-19 19:53 UTC
[LLVMdev] [RFC] Add empty() method to iterator_range.
On Wed, Mar 19, 2014 at 10:57 PM, Duncan P. N. Exon Smith < dexonsmith at apple.com> wrote:> > On Mar 19, 2014, at 11:32 AM, Aaron Ballman <aaron at aaronballman.com> > wrote: > > > On Wed, Mar 19, 2014 at 2:13 PM, Lang Hames <lhames at gmail.com> wrote: > >> Hi all, > >> > >> As RFCs go this is short and sweet - I think it would be nice to add an > >> empty method to iterator_range. Something like: > >> > >> bool empty() const { return begin() != end(); } > >> > >> My motivation is to make the 'if' test at the start of this common > pattern > >> tidier: > >> > >> if (!collection.empty()) > >> // initialization... > >> for (auto& c : collection) > >> // do something for each c. > >> > >> which I think that is preferable to: > >> > >> if (collection.begin() != collection.end()) > >> // initialization... > >> // for each... > >> > >> The empty method is just a single iterator comparison, so it should be > valid > >> and cheap for any underlying iterator type. > >> > >> Pros: > >> - Enables small aesthetic improvements. > >> - Would make iterator_range look slightly more "container-like", so it > could > >> substitute into more template code, though I'd expect templates that > take > >> read-only containers rather than iterators to be very rare in C++, and > >> templates that take collections and use only begin, end and empty to be > >> rarer still. > >> > >> Cons: ? > > I’m in favour. >+1. I also ran into this pattern, and suggested to add empty() method in r203354 review thread.> > > This is a pattern I ran into a handful of times when range-ifying > > parts of clang (but not so frequently that I felt it was a major win), > > so I'm roughly in favor of the API. However, if there's been > > standardization efforts for range, we should be sure that we're doing > > something compatible there. > > There are a couple of proposals [1][2] that I can find. It sounds like the > exact set of methods to provide is somewhat contentious [3]. empty() seems > pretty innocuous though. Note that the standard proposals use std::range > instead of std::iterator_range. > > In my opinion, other methods (e.g., front(), back(), and (for random access > iterators) operator[]) would also be useful. > > [1]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1871.html > [2]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3350.html > [3]: > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3513.html#range-arg > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-- Alexey Samsonov, MSK -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140319/2c9d4614/attachment.html>
Chandler Carruth
2014-Mar-20 21:31 UTC
[LLVMdev] [RFC] Add empty() method to iterator_range.
Since Richard first poked me about this, I'm still debating this. I'm torn in a bunch of different ways: 1) iterator_range is *not* a container. It shouldn't and can't be used like one. 2) members are really hard to extend. they make templates harder not easier because the widen the set of interfaces which *must* be on a given container 3) the concept of testing for emptiness is actually inherently useful for all ranges, unlike many other proposed extensions to the member interface, so maybe its OK to grow the interface in this direction 4) basing emptiness on iterators seems really bad because one of the important utilities of ranges is that they may expose concepts which are more flexible and efficient than a pair of iterators However, one thing makes me inclined to say "no" to this: the benefit is miniscule. It is too simple to just compare the iterators and not cross any of these bridges today. On Wed, Mar 19, 2014 at 11:57 AM, Duncan P. N. Exon Smith < dexonsmith at apple.com> wrote:> > This is a pattern I ran into a handful of times when range-ifying > > parts of clang (but not so frequently that I felt it was a major win), > > so I'm roughly in favor of the API. However, if there's been > > standardization efforts for range, we should be sure that we're doing > > something compatible there. > > There are a couple of proposals [1][2] that I can find. It sounds like the > exact set of methods to provide is somewhat contentious [3]. empty() seems > pretty innocuous though. Note that the standard proposals use std::range > instead of std::iterator_range. > > In my opinion, other methods (e.g., front(), back(), and (for random access > iterators) operator[]) would also be useful. >I think front, back, and [] are extremely contentious. =] While empty may be the most innocuous, it also seems the least useful today. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140320/3357c62e/attachment.html>
Hi Chandler, Agreed - ranges aren't containers, just views of sequences. Still, that's all many algorithms want, and it's just as valid to ask whether a sequence is empty as it is to ask that of a container. No contest on points 2 or 3, but I'm confused about point 4. When are ranges not pairs of iterators? I mean in a way that would clash with the proposal for empty() to be defined as '(begin(r) == end(r)' ? The benefit of doing this is similar in kind, though obviously less significant, to the benefit of range based for loops. It doesn't enable anything fundamentally new, but improving readability of C++ code is welcome. This is easy to punt (and I won't be especially bothered if that's what we do), but it also seems like it would be easy to implement our own version (llvm::is_empty?) and replace it when the committee decides on something, the same way we did with llvm::move? Is there any reason not to follow that approach? Cheers, Lang. On Thu, Mar 20, 2014 at 2:31 PM, Chandler Carruth <chandlerc at google.com>wrote:> Since Richard first poked me about this, I'm still debating this. > > I'm torn in a bunch of different ways: > > 1) iterator_range is *not* a container. It shouldn't and can't be used > like one. > 2) members are really hard to extend. they make templates harder not > easier because the widen the set of interfaces which *must* be on a given > container > 3) the concept of testing for emptiness is actually inherently useful for > all ranges, unlike many other proposed extensions to the member interface, > so maybe its OK to grow the interface in this direction > 4) basing emptiness on iterators seems really bad because one of the > important utilities of ranges is that they may expose concepts which are > more flexible and efficient than a pair of iterators > > However, one thing makes me inclined to say "no" to this: the benefit is > miniscule. It is too simple to just compare the iterators and not cross any > of these bridges today. > > On Wed, Mar 19, 2014 at 11:57 AM, Duncan P. N. Exon Smith < > dexonsmith at apple.com> wrote: > >> > This is a pattern I ran into a handful of times when range-ifying >> > parts of clang (but not so frequently that I felt it was a major win), >> > so I'm roughly in favor of the API. However, if there's been >> > standardization efforts for range, we should be sure that we're doing >> > something compatible there. >> >> There are a couple of proposals [1][2] that I can find. It sounds like >> the >> exact set of methods to provide is somewhat contentious [3]. empty() >> seems >> pretty innocuous though. Note that the standard proposals use std::range >> instead of std::iterator_range. >> >> In my opinion, other methods (e.g., front(), back(), and (for random >> access >> iterators) operator[]) would also be useful. >> > > I think front, back, and [] are extremely contentious. =] While empty may > be the most innocuous, it also seems the least useful today. > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140321/2db76925/attachment.html>