Creating a QListIterator over a temporary object?-CodePudding

Currently I'm doing some code reviews and stumbled on the following construct:

QVariantMap argumentMap = QJsonDocument::fromJson(" ... JSON-String ... ", &error).toVariant().toMap();
...

QListIterator<QVariant> keyIterator( argumentMap["key"].toList() );

while ( keyIterator.hasNext() ) ...

My first feeling was that the iterator is faulty here, as toList() returns a QVariantList by value resulting in a temporary object.

So the Ctor is defined as QListIterator(const QList<T> &list) and we found this [1]: "It is an official C feature to extend the life time of a temporary object to the life time of the const reference which refers to it." But first my argument was that life time of the const reference to the list is bound to the Ctor.

So I tried to dig deeper into the definition of QListIterator [2]:

Q_DECLARE_SEQUENTIAL_ITERATOR(List)

#define Q_DECLARE_SEQUENTIAL_ITERATOR(C) \
\
template <class T> \
class Q##C##Iterator \
{ \
    typedef typename Q##C<T>::const_iterator const_iterator; \
    Q##C<T> c; \
    const_iterator i; \
public: \
    inline Q##C##Iterator(const Q##C<T> &container) \
        : c(container), i(c.constBegin()) {} \

Now, I'm really confused! :) It seems that with the c member the Iterator holds it's own local copy of the list. So finally, I would say this usage is absolutely valid. Could someone please confirm this?

Plus, this construct is used all over the application and apparently never caused any problems.

Short addendum:

I found also this here [3]: "If you want to iterate over these using an STL iterator, you should always take a copy of the container and iterate over the copy. For example:"

// WRONG
QList<int>::const_iterator i;
for (i = splitter->sizes().begin(); i != splitter->sizes().end();   i)

First I thought this is the exact same problem, but on a second thought I would now say that the problem here is that begin() and end() are called on different copies of the list. Correct?

[1] https://blog.galowicz.de/2016/03/23/const_reference_to_temporary_object/

[2] https://code.woboq.org/qt5/qtbase/src/corelib/tools/qiterator.h.html

[3] https://doc.qt.io/qt-5/containers.html#stl-style-iterators

CodePudding user response：

The QListIterator should be fine, since it takes a copy of the list.

What you are referring to by the lifetime extension of temporaries is this:

{
auto const & myRef = foo.bar();  // returns by value so it returns a temporary
// you would expect the temporary to be gone now
// and myRef thus being a dangling reference, but it is not!
myRef.doSomething();  // perfectly fine
}
// now that myRef is out of scope also the temporary is destroyed

See here for a description of lifetimes.

However that is not relevant in this case because this mechanism can not extend the lifetime of a temporary to the lifetime of an object but only to the lifetime of a reference.

And yes, the last example is wrong for exactly the reason you gave: Comparing iterators to different (temporary) objects.