Reverse range adapter

Pete Cooper peter_cooper at apple.com
Tue Jul 28 10:19:25 PDT 2015


> On Jul 28, 2015, at 9:59 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> I'm OK calling it 'reverse' as you have (since it has just the one argument it shouldn't be ambiguous with the iterator versions)
> 
> * These functions shouldn't be ‘static’
Good point.  Made them inline like the other methods in the same file.
> * Could you try using non-member begin/end in the second version - that should allow it to work with arrays. Give it a go/add a test?
Done.  Added a test for this too.
> * Maybe test the case where a container has rbegin/rend and begin/end to ensure we still favor the rbegin/rend (and that it's not ambiguous?) - presumably they're more efficient, if they're provided?
Added a test for this too.  I left begin(), end() without method bodies so that if they were called we’d get linker errors.
> 
> & the reason you don't need explicit SFINAE is because you put the interesting expressions in the return type - so they're part of the SFINAE condition already, conveniently.
Makes sense.  Thanks for the explanation.
> 
> I think Saleem (cc'd) had an existing implementation of something like this that he might be willing to provide some insight from?
Cool.  Happy to see his implementation too, and to take whichever suits.

Updated patch included.

Cheers,
Pete


> 
> On Tue, Jul 28, 2015 at 9:43 AM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
> Hi David
> 
> Please find attached a patch for a reverse range adapter.  Its based on feedback you gave in http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20150720/289410.html <http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20150720/289410.html>.
> 
> There are 2 versions.  The first uses rbegin()/rend(), the second constructs std::reverse_iterators around begin()/end().
> 
> I was surprised to find I didn’t need enable_if or any other such tricks.
> 
> I’ve updated a single use of the pattern ‘for auto x : make_range(rbegin(), rend())’ to the new reverse method.
> 
> I was considering reverse_range instead as a name to avoid confusion with std::reverse.  I’d prefer to not do make_reverse_range just to save on characters.
> 
> Feedback welcome.
> 
> Cheers,
> Pete
> 
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150728/40730e6f/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: reverse.patch
Type: application/octet-stream
Size: 6699 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150728/40730e6f/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150728/40730e6f/attachment-0001.html>


More information about the llvm-commits mailing list