temporary object in range-based for

Question

I know that in general the life time of a temporary in a range-based for loop is extended to the whole loop (I've read C++11: The range-based for statement: "range-init" lifetime?). Therefore doing stuff like this is generally OK:

for (auto &thingy : func_that_returns_eg_a_vector())
  std::cout << thingy;

Now I'm stumbling about memory issues when I try to do something I thought to be similar with Qt's QList container:

#include <iostream>
#include <QList>

int main() {
  for (auto i : QList<int>{} << 1 << 2 << 3)
    std::cout << i << std::endl;
  return 0;
}

The problem here is that valgrind shows invalid memory access somewhere inside the QList class. However, modifying the example so that the list is stored in variable provides a correct result:

#include <iostream>
#include <QList>

int main() {
  auto things = QList<int>{} << 1 << 2 << 3;
  for (auto i : things)
    std::cout << i << std::endl;
  return 0;
}

Now my question is: am I doing something dumb in the first case resulting in e.g. undefined behaviour (I don't have enough experience reading the C++ standard in order to answer this for myself)? Or is this an issue with how I use QList, or how QList is implemented?

kennytm · Accepted Answer · 2012-04-14T13:02:52.670

12

Since you're using C++11, you could use initialization list instead. This will pass valgrind:

int main() {
  for (auto i : QList<int>{1, 2, 3})
    std::cout << i << std::endl;
  return 0;
}

The problem is not totally related to range-based for or even C++11. The following code demonstrates the same problem:

QList<int>& things = QList<int>() << 1;
things.end();

or:

#include <iostream>

struct S {
    int* x;

    S() { x = NULL; }
    ~S() { delete x; }

    S& foo(int y) {
        x = new int(y);
        return *this;
    }
};

int main() {
    S& things = S().foo(2);
    std::cout << *things.x << std::endl;
    return 0;
}

The invalid read is because the temporary object from the expression S() (or QList<int>{}) is destructed after the declaration (following C++03 and C++11 §12.2/5), because the compiler has no idea that the method foo() (or operator<<) will return that temporary object. So you are now refering to content of freed memory.

edited Apr 14 '12 at 13:02

answered Apr 14 '12 at 12:56

kennytm

510,854
105
1,084
1,005

Thanks for the clarification. And stupid me, of course I should have used an initialization list in the first place -- I somehow simply didn't think about it. Probably due to Qt's examples always using `<<` for similar cases. – Moritz Bunkus Apr 14 '12 at 13:03
Ah well, looks like C++11 support is only available in Qt 4.8 and later. But for cases such as this I can easily use the containers from the standard library instead. – Moritz Bunkus Apr 14 '12 at 13:15
Could this problem be circumvented by casting to `QList const&` (i.e. writing `for (auto i : static_cast const&>(QList{} << 1 << 2 << 3))`)? That way, it would be bound to a const reference in the `for` loop initialisation, if I read §6.5.4 correctly, and that in turn would extend the temporary’s life-time to the scope of the loop. – Konrad Rudolph Apr 14 '12 at 13:16
I cannot comment on the validity but valgrind says `no, you cannot` :) – Moritz Bunkus Apr 14 '12 at 13:26
1

The _real_ problem here is `QList`'s `operator<<` return type (non-const lvalue reference). `for (auto i : QList{})` without using `<<` is fine because the `QList` instance is an rvalue and will play nice with the semantics of `auto&&`. – ildjarn Apr 14 '12 at 16:06

score 7 · Answer 2 · answered Apr 14 '12 at 13:10

The compiler can't possibly know that the reference that is the result of three calls to operator << is bound to the temporary object QList<int>{}, so the life of the temporary is not extended. The compiler does not know (and can't be expected to know) anything about the return value of a function, except its type. If it's a reference, it doesn't know what it may bind to. I'm pretty sure that, in order for the life-extending rule to apply, the binding has to be direct.

This should work because the list is no longer a temporary:

#include <iostream>
#include <QList>

int main() {
  auto things = QList<int>{};
  for (auto i : things << 1 << 2 << 3)
    std::cout << i << std::endl;
  return 0;
}

And this should work because the binding is direct, so the rule can apply:

#include <iostream>
#include <QList>

int main() {
  for (auto i : QList<int>{1, 2, 3})
    std::cout << i << std::endl;
  return 0;
}

temporary object in range-based for

2 Answers2

Linked