(In reply to comment #0)
> Consider an iterator class that implements the requirements of a
> forward_iterator. We underlying datatype is always POD and the operator*()
> member of the iterator returns the results by value, since it is POD anyway.

The iterator requirements [iterator.iterators] say operator* must return iterator_traits<some_iterator>::reference

Created attachment 26300 [details]
Minimal example that reproduces bug, but with proper call-signature for operator*() (ie: returning reference)

Minimal example was changed with proper call-signature for operator*(), returning reference. 

The resulting behavior remains the same as explained in the original message though.

(In reply to comment #0)
> Headerfiles involved:
>  <iterator>

And <iostream>

> Consider an iterator class that implements the requirements of a
> forward_iterator.

Do you mean bidirectional iterator?  You can't reverse a forward iterator, and  std::reverse_iterator requires that its template parameter meets the requirements of a Bidirectional Iterator.
Your class doesn't implement those requirements, it doesn't have a post-increment operator.

And as I said in my previous comment, the iterator requirements say operator* returns the iterator's 'reference' type.  reverse_iterator::operator* then returns the same thing because the 'reference' type is, in effect, defined as "the type returned by operator*"

(Also, the name __traits is forbidden, names beginning with double-underscore are reserved for the implementation)

> The 'work-around' forces the use of forwarding. I put it as comment in the
> minimum example. If I should have attached a separate source file with just the
> operator*()-line replaced, I wish to express my apologies in advance.

The preprocessor strips comments out, so the workaround isn't in the attached file.


The first problem is that your class lies, saying its 'reference' type is a reference, but then returning something different from operator*.  That causes a reference to be bound to a temporary, as you'll see if you compile with -Wsystem-headers to enable warnings for code in the standard library headers.

The second problem is how reverse_iterator::operator* is defined:

      reference
      operator*() const
      {
	_Iterator __tmp = current;
	return *--__tmp;
      }

This creates a temporary iterator, then returns a reference to its d_val member. By the time that's written to std::cout the temporary has been destroyed.

For a bidirectional iterator the expression *r-- is required to be valid, so your type fails that requirement.

What you've written is not an iterator, it's a generator.  It doesn't meet the iterator requirements, so can't be used with std::reverse_iterator and std::copy and other templates that require iterators.

(In reply to comment #3)
> The preprocessor strips comments out, so the workaround isn't in the attached
> file.

Fixed by the new attachment 
 
> For a bidirectional iterator the expression *r-- is required to be valid, so
> your type fails that requirement.

Not fixed, this invalid.

actually, my bad, even if your type did meet the requirements it wouldn't work, and that *is* a libstdc++ bug

at some point between C++03 and C++11 the definition of reverse_iterator::operator* was changed to clarify this, making libstdc++'s implementation wrong.  The standard now says:

[ Note: This operation must use an auxiliary member variable rather than a temporary variable to avoid returning a reference that persists beyond the lifetime of its associated iterator. (See 24.2.) —end note ]

That change was present as early as the Feb 2004 draft, n1577

Also related: http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-closed.html#1052

Paolo started a discussion on this:
http://gcc.gnu.org/ml/libstdc++/2003-11/msg00089.html
but apparently the code wasn't changed. It is really sad that in most cases the size of reverse_iterator is needlessly doubled just so some rare fancy iterators can use it. And other fancy uses prevent from using a global instead (even a tls).

For the special case of a Iota_iterator, when the C++ iterator concept is eventually fixed (or for people who decide to ignore the requirement that reference be a reference), reference should be int, not int const&, so operator* would return by value.

Thanks, Marc, I'd failed to find the relevant DR number
http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#198

We could specialize reverse_iterator on a trait that says the extra member isn't needed, and then make that trait true for all iterator types defined in the library.  The size would still change for reverse_iterator<user_defined_iterator> but I don't see how else to meet the requirement.

(In reply to comment #8)
> We could specialize reverse_iterator on a trait that says the extra member
> isn't needed, and then make that trait true for all iterator types defined in
> the library.

I was just writing myself a note to try and get such a trait added to the standard ;-)
I don't know if it is worth doing that optimization now (I was complaining on the principle, I don't think I ever used reverse_iterator), depends on your motivation...

I can work on it for GCC 4.8

Created attachment 26323 [details]
implement DR 198

it's ugly, but it works

setting ABI keyword as this can't be properly fixed without an incompatible change

I think this bug is invalid, because the iterator being wrapped doesn't model ForwardIterator due to I found 24.2.5 [forward.iterators]/6, where it says:

  If a and b are both dereferenceable, then a == b if and only if *a and
  *b are bound to the same object.

Thanks, Dave.  So IIUC that means a counting iterator or other iterator that stores a value internally can only be an input iterator, and so can't be used with reverse_iterator.

Suspending pending resolution of Dave's issue:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3473.html#2204

Normative text vs. non-normative note == no contest, IMO.  But I guess it doesn't hurt to have the bug open if it doesn't mean any changes to the library.

The definition of reverse_iterator has been reverted to the C++03 version, which does not use an extra member, so "stashing iterators" can not be used with reverse_iterator.