| Trolltech Home | Qt-interest Home | Recent Threads | All Threads | Author | Date | |
| All threads index page 3 | |
I've run into a couple of problems with a template
class derived from QList<T>. I have the following code:
> template <typename T>
> class ManifestList : public ManifestElement, public QList<T>
> {
> public:
>
> ManifestList(const QString& name) : ManifestElement(name) {};
>
> void serialize_to_XML(QXmlStreamWriter &writer)
> {
> writer.writeStartElement(element_name());
> foreach (T element, *this)
> {
> element.serialize_to_XML(writer);
> }
> writer.writeEndElement();
> }
> };
>
> // Specialization for pointer types
>
> template <typename T>
> class ManifestList<T *> : public ManifestElement, public QList<T *>
> {
> public:
>
> ManifestList(const QString& name) : ManifestElement(name) {};
> ~ManifestList()
> {
> foreach (T *element, *this)
> {
> delete element;
> }
> }
>
> void serialize_to_XML(QXmlStreamWriter &writer)
> {
> writer.writeStartElement(element_name());
> foreach (T *element, *this)
> {
> element->serialize_to_XML(writer);
> }
> writer.writeEndElement();
> }
> };
1) When I compile this under VSC++ 2005, I get the following
warning:
> 'QSet<T> QList<T>::toSet(void) const' : no suitable definition
> provided for explicit template instantiation request
which seems to be related to the lack of a definition for toSet().
I'm confused about what's going on here - can someone explain ?
2. The code above segfaults somewhere in the foreach loop
of serialize_to_XML() but it's not clear to me why. This
occurs both with code compiled with g++ and VS so it looks
like my screw-up rather than a platform issue.
Can anyone see what I've got wrong ? I have some very similar
code where I reimplement the classes with a QList member, and
they work fine, but from a design POV it makes more sense to
derive from QList, if possible.
--
[ signature omitted ]
On Thursday 24 January 2008 13:46:37 Stephen Collyer wrote: > 1) When I compile this under VSC++ 2005, I get the following > > warning: > > 'QSet<T> QList<T>::toSet(void) const' : no suitable definition > > provided for explicit template instantiation request > > which seems to be related to the lack of a definition for toSet(). > I'm confused about what's going on here - can someone explain ? Yes: MSVC is a broken compiler by design. It requires you to do thing that aren't required by the C++ language. In other words, it's by *design* rejecting perfectly valid C++ code when building or using DLLs. You have to #include <QSet>. The reason for that is that when you instatiate a template class, it will try to instantiate ALL methods in that class. But it cannot instantiate the QList<T>::toSet() function because that is defined in qset.h. [I am not sure if gcc on Windows (mingw) is affected by the same brokenness. It might be the fault of the Windows executable format.] > 2. The code above segfaults somewhere in the foreach loop > of serialize_to_XML() but it's not clear to me why. This > occurs both with code compiled with g++ and VS so it looks > like my screw-up rather than a platform issue. Which of the two? Can you paste a backtrace and some line numbers? It's probably the pointer specialisation. Have you checked to see if the pointers in the list are valid? > Can anyone see what I've got wrong ? I have some very similar > code where I reimplement the classes with a QList member, and > they work fine, but from a design POV it makes more sense to > derive from QList, if possible. -- [ signature omitted ]
Attachment:
signature.asc
Description: This is a digitally signed message part.
Thiago Macieira wrote:
>> 2. The code above segfaults somewhere in the foreach loop
>> of serialize_to_XML() but it's not clear to me why. This
>> occurs both with code compiled with g++ and VS so it looks
>> like my screw-up rather than a platform issue.
>
> Which of the two? Can you paste a backtrace and some line numbers?
>
> It's probably the pointer specialisation. Have you checked to see if the
> pointers in the list are valid?
Yes, you're right, it is the pointer specialization. I should have said.
The pointers in the list seem to be entirely valid, according to
the VS debugger, and point to constructed objects of the appropriate
type.
Here's the stack trace, first from gdb:
> #0 0x00000021 in ?? ()
> #1 0x0804b938 in ~ManifestList (this=0xbfabf27c) at /code/mdp_qt/src/lib/ManifestList.h:135
> #2 0x0804ba79 in ~QForeachContainer (this=0xbfabf27c) at /usr/local/Trolltech/Qt-4.3.2/include/QtCore/qglobal.h:1828
> #3 0x0804b9ae in ~ManifestList (this=0xbfabf2e8) at /code/mdp_qt/src/lib/ManifestList.h:135
> #4 0x0804ba79 in ~QForeachContainer (this=0xbfabf2e8) at /usr/local/Trolltech/Qt-4.3.2/include/QtCore/qglobal.h:1828
> #5 0x0804bb8a in MDP::ManifestList<MDP::TransferWith*>::serialize_to_XML (this=0xbfabf394, writer=@0xbfabf348)
> at /code/mdp_qt/src/lib/ManifestList.h:146
and from VS:
> TransferWith.dll!MDP::ManifestList<MDP::TransferWith *>::~ManifestList<MDP::TransferWith *>() Line 135 + 0x1e bytes C++
> TransferWith.dll!QForeachContainer<MDP::ManifestList<MDP::TransferWith *> >::~QForeachContainer<MDP::ManifestList<MDP::TransferWith *> >() + 0x16 bytes C
> TransferWith.dll!MDP::ManifestList<MDP::TransferWith *>::~ManifestList<MDP::TransferWith *>() Line 137 C++
> TransferWith.dll!QForeachContainer<MDP::ManifestList<MDP::TransferWith *> >::~QForeachContainer<MDP::ManifestList<MDP::TransferWith *> >() + 0x16 bytes C
> TransferWith.dll!MDP::ManifestList<MDP::TransferWith *>::serialize_to_XML(QXmlStreamWriter & writer={...}) Line 148 C++
> ManifestElement.dll!MDP::ManifestElement::as_XML() Line 60 + 0x13 bytes C++
and here is the source:
> 123 template <typename T>
> 124 class ManifestList<T *> : public ManifestElement, public QList<T *>
> 125 {
> 126 public:
> 127
> 128 // Constructors and destructor
> 129
> 130 ManifestList(const QString& name) : ManifestElement(name) {};
> 131 ~ManifestList()
> 132 {
> 133 foreach (T *element, *this)
> 134 {
> 135 delete element;
> 136 }
> 137 }
> 138
> 139 // Public interface
> 140
> 141 void serialize_to_XML(QXmlStreamWriter &writer)
> 142 {
> 143 writer.writeStartElement(element_name());
> 144 foreach (T *element, *this)
> 145 {
> 146 element->serialize_to_XML(writer);
> 147 }
> 148 writer.writeEndElement();
> 149 }
> 150 };
--
[ signature omitted ]
On Thursday 24 January 2008 15:33:50 Stephen Collyer wrote:
> > 132 {
> > 133 foreach (T *element, *this)
> > 134 {
> > 135 delete element;
> > 136 }
> > 137 }
Just to be sure, replace your destructor's contents with:
qDeleteAll(*this);
--
[ signature omitted ]
Attachment:
signature.asc
Description: This is a digitally signed message part.
Thiago Macieira wrote:
> On Thursday 24 January 2008 15:33:50 Stephen Collyer wrote:
>>> 132 {
>>> 133 foreach (T *element, *this)
>>> 134 {
>>> 135 delete element;
>>> 136 }
>>> 137 }
>
> Just to be sure, replace your destructor's contents with:
>
> qDeleteAll(*this);
>
This makes no difference to the final result. I'm
still getting a core dump, though later on in the
code. What seems to be happening now is that the
~ManifestList() dtor executes, and execution continues
until, later on, QString::arg() tries to use a
presumably by-now-destructed member variable.
My main concern here is understanding why ~ManifestList()
is executing at all - I'm not sure if the class breaks
the foreach logic in some way, or if it's a bug somewhere
in Qt. The transition from the serialize_to_XML() method
to the dtor looks v. odd to me.
However, I've got an alternative version that works fine
based on a QList<T*> member, so unless you think that this
is worth pursuing from the POV of finding a potential bug,
I'm going to leave it as unsolved mystery for now, as I'm
short of time.
--
[ signature omitted ]
Stephen Collyer wrote:
>My main concern here is understanding why ~ManifestList()
>is executing at all - I'm not sure if the class breaks
>the foreach logic in some way, or if it's a bug somewhere
>in Qt. The transition from the serialize_to_XML() method
>to the dtor looks v. odd to me.
Ah, that might be the reason.
Your ManifestList container deletes its elements when it goes out of
scope. So the following code will crash:
void func()
{
ManifestList<Foo*> list1;
{
ManifestList<Foo*> list2;
list2.append(new Foo);
list1 = list2;
}
} // <--- crashes here
Notice how there are two lists which share the same elements. When the
first one (list2) goes out of scope, it deletes the elements. When the
second one goes out of scope, it deletes *again* the same elements. Hence
the crash.
Even if QList weren't implicitly shared, it still does not create a copy
of pointer elements when detaching.
You don't want a specialisation that does qDeleteAll(). You want a
QList<SmartPointer<Foo> >.
--
[ signature omitted ]
Attachment:
signature.asc
Description: This is a digitally signed message part.
Thiago Macieira wrote:
> Your ManifestList container deletes its elements when it goes out of
> scope. So the following code will crash:
>
> void func()
> {
> ManifestList<Foo*> list1;
> {
> ManifestList<Foo*> list2;
> list2.append(new Foo);
> list1 = list2;
> }
> } // <--- crashes here
>
> Notice how there are two lists which share the same elements. When the
> first one (list2) goes out of scope, it deletes the elements. When the
> second one goes out of scope, it deletes *again* the same elements. Hence
> the crash.
Right. And in my case, it's the damned foreach macro that's causing
the copying. Thanks, I see what's going on now. And I can't fix the
problem merely by iterating over the elements using a method that
doesn't take a copy of the list, since any user of the class may do so.
> You don't want a specialisation that does qDeleteAll(). You want a
> QList<SmartPointer<Foo> >.
And I guess that the particular SmartPointer would have to be
reference counted to avoid this particular problem so I'd guess
that QSharedDataPointer is a likely candidate.
OK, I see what's happening now and I'm happy to live with
the alternate version till I find the time to rework the
broken one.
--
[ signature omitted ]
On Friday 25 January 2008 10:24:13 Stephen Collyer wrote: > And I guess that the particular SmartPointer would have to be > reference counted to avoid this particular problem so I'd guess > that QSharedDataPointer is a likely candidate. Yes, QSharedDataPointer if you control the element. The element has to derive from QSharedData in order to have the atomic refcount. My implementation of QSmartPointer for generic pointers didn't make it into Qt 4.4. We are unsure of the need for this class at this time. It's similar to boost::shared_ptr (tr1::shared_pointer if your compiler already has TR1), so you can use that one if necessary. -- [ signature omitted ]
Attachment:
signature.asc
Description: This is a digitally signed message part.