Remove "delete value" from QSharedPointer
This allows a QSharedPointer to be used in contexts where the class in question is still forward-declared. This produced a warning in Qt 4 due to the expansion of the template, even if there was no chance of the pointer being deleted there (because the reference count could not drop to zero). Now, not only is the warning removed, but you can actually have the reference count drop to zero in a forward-declared class and it will do the right thing. That's because the deleter function is always recorded from the point of construction and we're sure that it wasn't forward-declared. The unit test for forward-declarations had to be rewritten. The previous version was passing only because the QSharedPointer object was created under the "tracking pointers" mode, which causes a custom deleter to be used in all cases. Task-number: QTBUG-25819 Change-Id: Ife37a4cea4551d94084b49ee03504dd39b8802c1 Reviewed-by: Lars Knoll <lars.knoll@nokia.com> Reviewed-by: Bradley T. Hughes <bradley.hughes@nokia.com>
This commit is contained in:
parent
6929fad9d9
commit
55c6f09b3a
@ -185,9 +185,7 @@ namespace QtSharedPointer {
|
||||
virtual inline ~ExternalRefCountData() { Q_ASSERT(!weakref.load()); Q_ASSERT(strongref.load() <= 0); }
|
||||
|
||||
// overridden by derived classes
|
||||
// returns false to indicate caller should delete the pointer
|
||||
// returns true in case it has already done so
|
||||
virtual inline bool destroy() { return false; }
|
||||
virtual inline void destroy() { }
|
||||
|
||||
#ifndef QT_NO_QOBJECT
|
||||
Q_CORE_EXPORT static ExternalRefCountData *getAndRef(const QObject *);
|
||||
@ -210,7 +208,7 @@ namespace QtSharedPointer {
|
||||
: destroyer(d)
|
||||
{ }
|
||||
|
||||
inline bool destroy() { destroyer(this); return true; }
|
||||
inline void destroy() { destroyer(this); }
|
||||
inline void operator delete(void *ptr) { ::operator delete(ptr); }
|
||||
inline void operator delete(void *, void *) { }
|
||||
};
|
||||
@ -327,31 +325,17 @@ namespace QtSharedPointer {
|
||||
typedef ExternalRefCountData Data;
|
||||
|
||||
inline void deref()
|
||||
{ deref(d, this->value); }
|
||||
static inline void deref(Data *d, T *value)
|
||||
{ deref(d); }
|
||||
static inline void deref(Data *d)
|
||||
{
|
||||
if (!d) return;
|
||||
if (!d->strongref.deref()) {
|
||||
if (!d->destroy())
|
||||
delete value;
|
||||
d->destroy();
|
||||
}
|
||||
if (!d->weakref.deref())
|
||||
delete d;
|
||||
}
|
||||
|
||||
inline void internalConstruct(T *ptr)
|
||||
{
|
||||
#ifdef QT_SHAREDPOINTER_TRACK_POINTERS
|
||||
internalConstruct<void (*)(T *)>(ptr, normalDeleter);
|
||||
#else
|
||||
if (ptr)
|
||||
d = new Data;
|
||||
else
|
||||
d = 0;
|
||||
internalFinishConstruction(ptr);
|
||||
#endif
|
||||
}
|
||||
|
||||
template <typename Deleter>
|
||||
inline void internalConstruct(T *ptr, Deleter deleter)
|
||||
{
|
||||
@ -381,8 +365,6 @@ namespace QtSharedPointer {
|
||||
inline ExternalRefCount() : d(0) { }
|
||||
inline ExternalRefCount(Qt::Initialization i) : Basic<T>(i) { }
|
||||
|
||||
inline ExternalRefCount(T *ptr) : Basic<T>(Qt::Uninitialized) // throws
|
||||
{ internalConstruct(ptr); }
|
||||
template <typename Deleter>
|
||||
inline ExternalRefCount(T *ptr, Deleter deleter) : Basic<T>(Qt::Uninitialized) // throws
|
||||
{ internalConstruct(ptr, deleter); }
|
||||
@ -403,7 +385,7 @@ namespace QtSharedPointer {
|
||||
other.ref();
|
||||
qSwap(d, o);
|
||||
qSwap(this->value, actual);
|
||||
deref(o, actual);
|
||||
deref(o);
|
||||
}
|
||||
|
||||
inline void internalSwap(ExternalRefCount &other)
|
||||
@ -448,7 +430,7 @@ namespace QtSharedPointer {
|
||||
this->value = 0;
|
||||
|
||||
// dereference saved data
|
||||
deref(o, actual);
|
||||
deref(o);
|
||||
}
|
||||
|
||||
Data *d;
|
||||
@ -463,7 +445,7 @@ public:
|
||||
inline QSharedPointer() { }
|
||||
// inline ~QSharedPointer() { }
|
||||
|
||||
inline explicit QSharedPointer(T *ptr) : BaseClass(ptr) // throws
|
||||
inline explicit QSharedPointer(T *ptr) : BaseClass(ptr, &QtSharedPointer::normalDeleter<T>) // throws
|
||||
{ }
|
||||
|
||||
template <typename Deleter>
|
||||
|
@ -1,52 +0,0 @@
|
||||
/****************************************************************************
|
||||
**
|
||||
** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies).
|
||||
** Contact: http://www.qt-project.org/
|
||||
**
|
||||
** This file is part of the test suite of the Qt Toolkit.
|
||||
**
|
||||
** $QT_BEGIN_LICENSE:LGPL$
|
||||
** GNU Lesser General Public License Usage
|
||||
** This file may be used under the terms of the GNU Lesser General Public
|
||||
** License version 2.1 as published by the Free Software Foundation and
|
||||
** appearing in the file LICENSE.LGPL included in the packaging of this
|
||||
** file. Please review the following information to ensure the GNU Lesser
|
||||
** General Public License version 2.1 requirements will be met:
|
||||
** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
|
||||
**
|
||||
** In addition, as a special exception, Nokia gives you certain additional
|
||||
** rights. These rights are described in the Nokia Qt LGPL Exception
|
||||
** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
|
||||
**
|
||||
** GNU General Public License Usage
|
||||
** Alternatively, this file may be used under the terms of the GNU General
|
||||
** Public License version 3.0 as published by the Free Software Foundation
|
||||
** and appearing in the file LICENSE.GPL included in the packaging of this
|
||||
** file. Please review the following information to ensure the GNU General
|
||||
** Public License version 3.0 requirements will be met:
|
||||
** http://www.gnu.org/copyleft/gpl.html.
|
||||
**
|
||||
** Other Usage
|
||||
** Alternatively, this file may be used in accordance with the terms and
|
||||
** conditions contained in a signed written agreement between you and Nokia.
|
||||
**
|
||||
**
|
||||
**
|
||||
**
|
||||
**
|
||||
**
|
||||
** $QT_END_LICENSE$
|
||||
**
|
||||
****************************************************************************/
|
||||
|
||||
#define QT_SHAREDPOINTER_TRACK_POINTERS
|
||||
#include "qsharedpointer.h"
|
||||
|
||||
class ForwardDeclared;
|
||||
ForwardDeclared *forwardPointer();
|
||||
|
||||
void externalForwardDeclaration()
|
||||
{
|
||||
struct Wrapper { QSharedPointer<ForwardDeclared> pointer; };
|
||||
}
|
||||
|
@ -1,6 +1,7 @@
|
||||
/****************************************************************************
|
||||
**
|
||||
** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies).
|
||||
** Copyright (C) 2012 Intel Corporation.
|
||||
** Contact: http://www.qt-project.org/
|
||||
**
|
||||
** This file is part of the test suite of the Qt Toolkit.
|
||||
@ -40,10 +41,17 @@
|
||||
****************************************************************************/
|
||||
|
||||
#include "forwarddeclared.h"
|
||||
#include "qsharedpointer.h"
|
||||
|
||||
ForwardDeclared *forwardPointer()
|
||||
class ForwardDeclared
|
||||
{
|
||||
return new ForwardDeclared;
|
||||
public:
|
||||
~ForwardDeclared();
|
||||
};
|
||||
|
||||
QSharedPointer<ForwardDeclared> *forwardPointer()
|
||||
{
|
||||
return new QSharedPointer<ForwardDeclared>(new ForwardDeclared);
|
||||
}
|
||||
|
||||
int forwardDeclaredDestructorRunCount;
|
||||
|
@ -1,6 +1,7 @@
|
||||
/****************************************************************************
|
||||
**
|
||||
** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies).
|
||||
** Copyright (C) 2012 Intel Corporation.
|
||||
** Contact: http://www.qt-project.org/
|
||||
**
|
||||
** This file is part of the test suite of the Qt Toolkit.
|
||||
@ -43,12 +44,17 @@
|
||||
#define FORWARDDECLARED_H
|
||||
|
||||
extern int forwardDeclaredDestructorRunCount;
|
||||
class ForwardDeclared
|
||||
{
|
||||
public:
|
||||
~ForwardDeclared();
|
||||
};
|
||||
class ForwardDeclared;
|
||||
|
||||
ForwardDeclared *forwardPointer();
|
||||
#ifdef QT_NAMESPACE
|
||||
namespace QT_NAMESPACE {
|
||||
#endif
|
||||
template <typename T> class QSharedPointer;
|
||||
#ifdef QT_NAMESPACE
|
||||
}
|
||||
using namespace QT_NAMESPACE;
|
||||
#endif
|
||||
|
||||
QSharedPointer<ForwardDeclared> *forwardPointer();
|
||||
|
||||
#endif // FORWARDDECLARED_H
|
||||
|
@ -4,7 +4,6 @@ TARGET = tst_qsharedpointer
|
||||
QT = core testlib
|
||||
|
||||
SOURCES = tst_qsharedpointer.cpp \
|
||||
forwarddeclaration.cpp \
|
||||
forwarddeclared.cpp \
|
||||
wrapper.cpp
|
||||
|
||||
|
@ -1,6 +1,7 @@
|
||||
/****************************************************************************
|
||||
**
|
||||
** Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies).
|
||||
** Copyright (C) 2012 Intel Corporation.
|
||||
** Contact: http://www.qt-project.org/
|
||||
**
|
||||
** This file is part of the test suite of the Qt Toolkit.
|
||||
@ -48,6 +49,7 @@
|
||||
#include <QtCore/QVector>
|
||||
|
||||
#include "externaltests.h"
|
||||
#include "forwarddeclared.h"
|
||||
#include "wrapper.h"
|
||||
|
||||
#include <stdlib.h>
|
||||
@ -68,9 +70,9 @@ private slots:
|
||||
void basics();
|
||||
void operators();
|
||||
void swap();
|
||||
void forwardDeclaration1();
|
||||
void forwardDeclaration2();
|
||||
void useOfForwardDeclared();
|
||||
void memoryManagement();
|
||||
void dropLastReferenceOfForwardDeclared();
|
||||
void downCast();
|
||||
void functionCallDownCast();
|
||||
void upCast();
|
||||
@ -341,43 +343,12 @@ void tst_QSharedPointer::swap()
|
||||
QVERIFY(*p1 == 42);
|
||||
}
|
||||
|
||||
class ForwardDeclared;
|
||||
ForwardDeclared *forwardPointer();
|
||||
void externalForwardDeclaration();
|
||||
extern int forwardDeclaredDestructorRunCount;
|
||||
|
||||
void tst_QSharedPointer::forwardDeclaration1()
|
||||
void tst_QSharedPointer::useOfForwardDeclared()
|
||||
{
|
||||
#if defined(Q_CC_SUN) || defined(Q_CC_WINSCW) || defined(Q_CC_RVCT)
|
||||
QSKIP("This type of forward declaration is not valid with this compiler");
|
||||
#else
|
||||
externalForwardDeclaration();
|
||||
|
||||
struct Wrapper { QSharedPointer<ForwardDeclared> pointer; };
|
||||
|
||||
forwardDeclaredDestructorRunCount = 0;
|
||||
{
|
||||
Wrapper w;
|
||||
w.pointer = QSharedPointer<ForwardDeclared>(forwardPointer());
|
||||
QVERIFY(!w.pointer.isNull());
|
||||
}
|
||||
QCOMPARE(forwardDeclaredDestructorRunCount, 1);
|
||||
#endif
|
||||
// this just a compile test: use the forward-declared class
|
||||
QSharedPointer<ForwardDeclared> sp;
|
||||
}
|
||||
|
||||
#include "forwarddeclared.h"
|
||||
|
||||
void tst_QSharedPointer::forwardDeclaration2()
|
||||
{
|
||||
forwardDeclaredDestructorRunCount = 0;
|
||||
{
|
||||
struct Wrapper { QSharedPointer<ForwardDeclared> pointer; };
|
||||
Wrapper w1, w2;
|
||||
w1.pointer = QSharedPointer<ForwardDeclared>(forwardPointer());
|
||||
QVERIFY(!w1.pointer.isNull());
|
||||
}
|
||||
QCOMPARE(forwardDeclaredDestructorRunCount, 1);
|
||||
}
|
||||
|
||||
void tst_QSharedPointer::memoryManagement()
|
||||
{
|
||||
@ -444,6 +415,15 @@ void tst_QSharedPointer::memoryManagement()
|
||||
QCOMPARE(ptr.data(), (Data*)0);
|
||||
}
|
||||
|
||||
void tst_QSharedPointer::dropLastReferenceOfForwardDeclared()
|
||||
{
|
||||
// pointer to shared-pointer is weird, but we need to do it so that
|
||||
// we can drop the last reference in a different .cpp than where it was created
|
||||
forwardDeclaredDestructorRunCount = 0;
|
||||
delete forwardPointer();
|
||||
QCOMPARE(forwardDeclaredDestructorRunCount, 1);
|
||||
}
|
||||
|
||||
class DerivedData: public Data
|
||||
{
|
||||
public:
|
||||
@ -1715,17 +1695,6 @@ void tst_QSharedPointer::invalidConstructs_data()
|
||||
"ptr = new Data;";
|
||||
|
||||
// use of forward-declared class
|
||||
QTest::newRow("forward-declaration")
|
||||
#ifdef Q_CC_CLANG
|
||||
// Deleting a forward declaration is undefined, which results in a linker error with clang
|
||||
<< &QTest::QExternalTest::tryLinkFail
|
||||
#else
|
||||
// Other compilers accept the code, but do not call the destructor at run-time
|
||||
<< &QTest::QExternalTest::tryRun
|
||||
#endif
|
||||
<< "forwardDeclaredDestructorRunCount = 0;\n"
|
||||
"{ QSharedPointer<ForwardDeclared> ptr = QSharedPointer<ForwardDeclared>(forwardPointer()); }\n"
|
||||
"exit(forwardDeclaredDestructorRunCount);";
|
||||
QTest::newRow("creating-forward-declaration")
|
||||
<< &QTest::QExternalTest::tryCompileFail
|
||||
<< "QSharedPointer<ForwardDeclared>::create();";
|
||||
@ -1839,7 +1808,6 @@ void tst_QSharedPointer::invalidConstructs()
|
||||
|
||||
QTest::QExternalTest test;
|
||||
test.setQtModules(QTest::QExternalTest::QtCore);
|
||||
test.setExtraProgramSources(QStringList() << QFINDTESTDATA("forwarddeclared.cpp"));
|
||||
test.setProgramHeader(
|
||||
"#define QT_SHAREDPOINTER_TRACK_POINTERS\n"
|
||||
"#define QT_DEBUG\n"
|
||||
@ -1849,9 +1817,7 @@ void tst_QSharedPointer::invalidConstructs()
|
||||
"struct Data { int i; };\n"
|
||||
"struct DerivedData: public Data { int j; };\n"
|
||||
"\n"
|
||||
"extern int forwardDeclaredDestructorRunCount;\n"
|
||||
"class ForwardDeclared;\n"
|
||||
"ForwardDeclared *forwardPointer();\n"
|
||||
);
|
||||
|
||||
QFETCH(QString, code);
|
||||
|
Loading…
Reference in New Issue
Block a user