[llvm] r243581 - Reapply "Add reverse(ContainerTy) range adapter."

[Pete Cooper via llvm-commits]

Thanks for the tests Duncan.  I’ve committed a fix, along with your tests, in r278991.

Cheers,
Pete
> On Aug 17, 2016, at 12:13 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> 
> [resending to the "new" llvm-commits!]
> 
> [A little necromancy here...]
> 
> I think Charles was correct.
> 
> The attached rbegin.patch adds const-qualified rbegin() overloads for ReverseOnlyVector and BidirectionalVector.
> 
> It fails to compile, because the `has_rbegin` type trait returns false when rbegin() is const-overloaded.  reverse() tries to call begin()/end() on ReverseOnlyVector and they don't exist.
> 
> If you remove ReverseOnlyVector from the list of types to test, then it compiles.  However, all three types tested fail the new HasRbegin test.
> 
> I hit this because I'm working on a patch that makes the ilist reverse iterators incompatible with std::reverse_iterator.
> 
> I haven't made time to fix the type trait yet (or I'd commit this patch with the fix), but I thought I'd post the problem in the meantime.
> 
> <rbegin.patch>
> 
>> On 2015-Jul-29, at 20:32, Pete Cooper <peter_cooper at apple.com> wrote:
>> 
>> 
>> 
>> Sent from my iPhone
>> 
>>> On Jul 29, 2015, at 7:56 PM, Charles Davis <cdavis5x at gmail.com> wrote:
>>> 
>>> 
>>>> On Jul 29, 2015, at 6:48 PM, Pete Cooper <peter_cooper at apple.com> wrote:
>>>> 
>>>> Should be fixed by r243581.  Sorry for the breakage.
>>> That fixed the build error, alright (and thanks for the quick turnaround!), but it doesn’t really address the “has_rbegin not quite working right” concern I brought up. I guess it’s OK for `std::vector`, but I’m just afraid we might get strange or broken behavior from this for other containers with overloaded-for-const `rbegin()`/`rend()` methods where `std::reverse_iterator` might not work right. I don’t know if there are any that we need to worry about, though.
>> Sorry, should have answered this part before. I was in a rush to get the fix in.
>> 
>> So I believe it'll be ok because the ContainerTy itself knows if it's const or not so will select the correct method. From your log:
>> 
>> include/llvm/ADT/STLExtras.h:225:6: note: candidate template ignored: substitution failure [with ContainerTy = const std::__1::vector<const llvm::GlobalValue *, std::__1::allocator<const llvm::GlobalValue *> > &]: call to 'make_reverse_iterator' is ambiguous
>> auto reverse(
>> 
>> So hopefully it's all good, but thanks for looking in to it.
>> 
>> Pete
>>> 
>