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: ?
Cheers,
Lang.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140319/99da0e4c/attachment.html>
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: ?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. ~Aaron
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
On 19/03/14 19:13, Lang Hames 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(); } >I think this should be bool empty() const { return begin() == end(); } Otherwise, seems useful to me! - Roel> 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: ? > > Cheers, > Lang. > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >
On Wed, Mar 19, 2014 at 11:13 AM, 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.For this reason I'd personally consider writing this as a free function over ranges: template<typename T> bool is_empty(T t) { return begin(t) == end(t); } or the like. Why bother reimplementing empty in every range/container (& eventually we'd do this in cases where we used external adaptation to rangify a non-range type (though that'll be rare))> 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++,This is, hopefully, going to become exceedingly common, actually - working with ranges is much more convenient than always passing begin/end (as you've seen with empty) and in the end we hope to have a lot of range-based algorithms and more range-based adapters.> and > templates that take collections and use only begin, end and empty to be > rarer still.This would be true, though - currently the range concept is just a thing that you can call begin(r) and end(r) on, it's unlikely to be broadened (which is why range-based templates would be more likely to use something like is_empty as shown above).> Cons: ?I haven't gone through all the linked threads just yet (some other replies pointed out a few prior threads that touched on range algorithms and empty specifically) but I recall Chandler having some objections to diving into this sort of thing too quickly as there's some standardization efforts that he would like to consider before heading down this path. I'm not personally too fussed about that, but figured I'd mention it. - David
Hi All, Thanks for the feedback. CC'ing cfe-dev, since they may have thoughts on this too.> I think this should be > > bool empty() const { return begin() == end(); }Yes. Yes it should. Though you have to admit, my version would have sped up a lot of passes in the general case: if (Fn.blocks.empty())... ;) For this reason I'd personally consider writing this as a free> function over ranges: > > template<typename T> > bool is_empty(T t) { return begin(t) == end(t); } >I have a knee-jerk dislike of free functions, just because they break with the existing container interface. That said they have two concrete advantages: 1) With a free function you get 'empty' for free if you implement a new range type (e.g. array_range). 2) You can specialize is_empty for types where (for whatever bizarre reason) testing emptiness is cheap but constructing iterators is expensive. Whether that's worth breaking the existing idiom for, I don't know. I'm going to have to have a read through Duncan's links to get some more perspective (thanks for those Duncan!). - Lang. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140320/82af2394/attachment.html>