Search code examples
c++optimizationiteratorlazy-evaluationgcc4.9

Compiler optimization breaks lazy iterator


I wrote a custom container with its custom iterator. Due to the specific features of the container the iterator must be evaluated lazily. For the sake of the question the relevant part of the code is the dereferencing operator of the iterator which is implemented this way

template<typename T>
struct Container
{
  vector<T> m_Inner;

  // This should calculate the appropriate value.
  // In this example is taken from a vec but in 
  //the real use-case is calculated on request
  T Value(int N)
  { m_Inner.at(N); }
}

template<typename T>
struct Lazy_Iterator
{
  mutable pair<int, T> m_Current;
  int Index
  Container<T>* C

  Lazy_Iterator(const Container& Cont, int N):
    m_Current{Index, T{}}, Index{N}, C{&Cont}
  {      }

  pair<int, T>&
  operator*() const // __attribute__((noinline)) (this cures the symptom)
  {
      m_Current.first = Index; /// Optimized out
      m_Current.second = C->Value(Index); /// Optimized out
      return m_Current;
  }

}

Because the iterator itself is a template, its functions can be freely inlined by the compiler.

When I compile the code without optimizations the returned value is updated as expected. When I use the release compiler optimization (-O2 in GCC 4.9) in some cases the compiler optimizes out the lines that I marked as Optimized out even though the m_Current member is marked as mutable. As a consequence the return value does not match the value that the iterator should be pointing at.

Is this an expected behavior? Do you know any portable way to specify that the content of that function should be evaluated even if it's marked const?

I hope the question is exhaustive enough to be useful. Please advice if more details could be helpful in this case.

Edit:

To answer one comment, this is a potential usage taken from a small test program:

Container<double> myC;
Lazy_Iterator<double> It{myC, 0}
cout << "Creation: " << it->first << " , " << it->second << endl;

auto it2 = it;
cout << "Copy: "<<  it2->first << " , " << it2->second << endl;

cout << "Pre-increment: " << (it++)->first << " , " << it->second << endl;
cout << "Post-increment: " << (++it)->first << " , " << it->second << endl;
cout << "Pre-decrement: " << (it--)->first << " , " << it->second << endl;
cout << "Post-decrement: " << (--it)->first << " , " << it->second << endl;
cout << "Iterator addition: " << (it+2)->first << " , " << (it+2)->second << endl;
cout << "Iterator subtraction: "<< (it-2)->first << " , " << (it-2)->second << endl;

reverse_iterator<Lazy_Iterator> rit{it};
cout << "Reverse Iterator: " << rit->first << " , " << rit->second << endl;

auto rit2 = rit;
cout << "Reverse Iterator copy: " << rit2->first << " , " << rit2->second << endl;

cout << "Rev Pre-increment: " << (rit++)->first << " , " << rit->second << endl;
cout << "Rev Post-increment: " << (++rit)->first << " , " << rit->second << endl;
cout << "Rev Pre-decrement: " << (rit--)->first << " , " << rit->second << endl;
cout << "Rev Post-decrement: " << (--rit)->first << " , " << rit->second << endl;
cout << "Rev Iterator addition: " << (rit+2)->first << " , " << (rit+2)->second << endl;
cout << "Rev Iterator subtraction: "<< (rit-2)->first << " , " << (rit-2)->second << endl;

The test results are as expected for all tests except the last two lines

The last two lines of the test break down when the optimization is turned on.

The system actually work well and is not so more dangerous than any other iterator. Of course it will fail if the container gets deleted under his nose and it's probably safer to use the returned value by copy and not just keep the reference around, but this is off-topic


Solution

  • There is a problem with difference between physical iterator held by reverse_iterator (what is returned by .base()) and logical value it points to: they are off-by-one. reverse_iterator might do return *(--internal_iterator); on dereference, which leaves you with dangling reference to internals of destroyed function-local temporary.

    After another reading of standard, I found out that it has additional requirements to avoid such scenario, read note.

    Also I found that GCC 4.9 standard library is non-compliant. It uses a temporary. So, I think it is a GCC bug.

    Edit: Standard quote

    24.5.1.3.4 operator*     [reverse.iter.op.star]

    reference operator*() const;
    

    1 Effects:

    deref_tmp = current;  
    --deref_tmp; 
    return *deref_tmp;
    

    2 [ 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 ]

    Follow-up reading: Library Defect Report 198.

    And it seems that it is returned to old behaviour.

    Late edit: P0031 was voted in C++17 Working Draft. It states that reverse_iterator uses temporary, not member to hold intermediate value.