[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:

> 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.

Ok, I missed this comment; I thought it applied to the change - not the proposed fix.
I disagree. I do not believe that putting the instance variable back and not using it will not break code across TUs.

The only use of the instance variable __t is in the operator*() for the reverse iterator, which used to say:
	_LIBCPP_INLINE_VISIBILITY reference operator*() const {__t = current; return *--__t;}
(i.e, the variable is set immediately before use)
but now says:
	_LIBCPP_INLINE_VISIBILITY reference operator*() const {_Iter __tmp = current; return *--__tmp;}

The purpose of having this instance variable “stick around”, rather than being a local variable, was to support iterators whose operator* returned a reference to something inside of the iterator. If the iterator was a local variable, then the things contained inside of it would be destroyed when the iterator was destroyed.

However, in #2360, STL at microsoft.com pointed out that this was inviting silent data races, and the LWG decided not to support those kind of iterators any more.
(see the comments on LWG issue #198, which #2360 partially reversed).

Richard — clearly you think otherwise.
I’d like to hear your explanation as to why.

— Marshall

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