[libcxx] r203587 - Implement LWG 2360: 'reverse_iterator::operator*() is unimplementable'. Note that this is a (small) behavior change in the library. Reverse iterators whose base iterators' operator* return references to 'within themselves' have been sacrificed to the greater goal of avoiding data races.

[Marshall Clow]

On Mar 11, 2014, at 2:14 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Tue, Mar 11, 2014 at 2:00 PM, Marshall Clow <mclow.lists at gmail.com> wrote:
> On Mar 11, 2014, at 10:32 AM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
>>  {
>> -private:
>> -    mutable _Iter __t;
>> 
>> This looks like an ABI break; is that OK?
> 
> Sigh. Yes; yes, it is.
> This changes the size of a reverse_iterator.
> 
> Thanks for catching this, Richard. Apparently I was asleep at the wheel.
> 
> I’m open to suggestions here:
> a) I can put the instance variable back, and never use it. No ABI break, but an unused iterator in every reverse iterator.
> 
> Any code that passes a reverse_iterator between TUs built against different libc++ versions would still not work.

Absolutely.

> 
> b) I can leave it the way it is, and say - yeah, it’s an ABI break. Fortunately, I suspect that very few people hvea a reverse_iterator stored in a data file somewhere.
> c) Something else.. ?
> 
> You could push the choice onto the user: leave it how it was in std::__1,  add a "correct" reverse_iterator in std::__2, and give people an option to pick whether they want the newest-available version or the ABI-stable version (maybe selected by a macro).

Yes; that’s what the versioned namespaces are supposed to allow.


> (Hopefully this will clarify what I mean:
> 
> #ifndef _LIBCPP_UNSTABLE_ABI
> _LIBCPP_BEGIN_NAMESPACE_STD // std::__1
>   // old reverse_iterator
> _LIBCPP_END_NAMESPACE_STD
> #else
> _LIBCPP_BEGIN_NAMESPACE_STD_UNSTABLE // std::__2
>   // new reverse_iterator
> _LIBCPP_BEGIN_NAMESPACE_STD_UNSTABLE
> #endif
> 
> ... or even:
> 
> _LIBCPP_BEGIN_NAMESPACE_STD_MULTI_ABI // std::__1 or std::__2 depending on UNSTABLE_ABI macro
> class reverse_iterator { //...
> #ifdef _LIBCPP_UNSTABLE_ABI
> private:
>   mutable _Iter __t;
> #endif
> // ...
> };
> _LIBCPP_END_NAMESPACE_STD_MULTI_ABI
> 
> ... with the intent that the __2 ABI is subject to change for now, but when any platform next gets to do an ABI break, we define __2 as the new stable ABI, and use __3 as the next unstable ABI).

Yes, I want to do this - but I think it’s overkill for this.
I’m leaning towards just putting the instance variable back.
(and noting this for future ABI break/versioned namespace)

Thanks again for pointing this out.

— Marshall





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140311/f95836ef/attachment.html>