CODEWITHSUNDEEP
c++qtc++11
user507077
user507077

Reputation:

temporary object in range-based for

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?

Upvotes: 18

Views: 2816

Answers (2)

cvoinescu
cvoinescu

Reputation: 856

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;
}

Upvotes: 6

kennytm
kennytm

Reputation: 523494

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.

Upvotes: 11

Related Questions