From e940872c12399e8fc5c33ed2e52a719373bc03cf Mon Sep 17 00:00:00 2001
From: Andreas Beckermann <beckermann@monument-software.de>
Date: Thu, 17 Oct 2019 08:49:52 +0200
Subject: Fix hasVirtualDestructor() for implicit destructors

If a derived class does not declare an explicit destructor,
AbstractMetaClass::hasVirtualDestructor() is set to false.
However this is incorrect for classes that have a base class that in
turn provide a virtual destructor.

Consequently shiboken generates incorrect code in cases where it depends
on that value (e.g. for methods that override ownership of the returned
object - instead of releasing ownership, shiboken invalidates it).

This patch considers the baseClass()->hasVirtualDestructor() after
traversing all functions (which checks for explicit destructors).

Change-Id: Ifc4a472290d835dd6ef7d702b912643c9a2b42da
Reviewed-by: Friedemann Kleint <Friedemann.Kleint@qt.io>
---
 .../shiboken2/ApiExtractor/abstractmetabuilder.cpp |  6 +++-
 .../ApiExtractor/tests/testdtorinformation.cpp     | 33 ++++++++++++++++++++++
 .../ApiExtractor/tests/testdtorinformation.h       |  1 +
 3 files changed, 39 insertions(+), 1 deletion(-)

(limited to 'sources')

diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
index 6aafe68be..84721968d 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
@@ -512,8 +512,12 @@ void AbstractMetaBuilderPrivate::traverseDom(const FileModelItem &dom)
 
     ReportHandler::startProgress("Fixing class inheritance...");
     for (AbstractMetaClass *cls : qAsConst(m_metaClasses)) {
-        if (!cls->isInterface() && !cls->isNamespace())
+        if (!cls->isInterface() && !cls->isNamespace()) {
             setupInheritance(cls);
+            if (!cls->hasVirtualDestructor() && cls->baseClass()
+                && cls->baseClass()->hasVirtualDestructor())
+                cls->setHasVirtualDestructor(true);
+        }
     }
 
     ReportHandler::startProgress("Detecting inconsistencies in class model...");
diff --git a/sources/shiboken2/ApiExtractor/tests/testdtorinformation.cpp b/sources/shiboken2/ApiExtractor/tests/testdtorinformation.cpp
index f4204e9b9..0eee8af24 100644
--- a/sources/shiboken2/ApiExtractor/tests/testdtorinformation.cpp
+++ b/sources/shiboken2/ApiExtractor/tests/testdtorinformation.cpp
@@ -69,6 +69,39 @@ void TestDtorInformation::testDtorIsVirtual()
     QCOMPARE(AbstractMetaClass::findClass(classes, QLatin1String("Subject"))->hasVirtualDestructor(), true);
 }
 
+void TestDtorInformation::testDtorFromBaseIsVirtual()
+{
+    const char* cppCode = R"CPP(class ControlBase { public: ~ControlBase() {} };
+class Control : public ControlBase {};
+class SubjectBase { public: virtual ~SubjectBase() {} };
+class Subject : public SubjectBase {};
+)CPP";
+    const char* xmlCode = R"XML(<typesystem package="Foo"><value-type name="ControlBase"/>
+<value-type name="Control"/>"
+<value-type name="SubjectBase"/>"
+<value-type name="Subject"/>
+</typesystem>
+)XML";
+    QScopedPointer<AbstractMetaBuilder> builder(TestUtil::parse(cppCode, xmlCode));
+    QVERIFY(!builder.isNull());
+    AbstractMetaClassList classes = builder->classes();
+    QCOMPARE(classes.count(), 4);
+
+    auto klass = AbstractMetaClass::findClass(classes, QLatin1String("ControlBase"));
+    QVERIFY(klass);
+    QVERIFY(!klass->hasVirtualDestructor());
+    klass = AbstractMetaClass::findClass(classes, QLatin1String("Control"));
+    QVERIFY(klass);
+    QVERIFY(!klass->hasVirtualDestructor());
+
+    klass = AbstractMetaClass::findClass(classes, QLatin1String("SubjectBase"));
+    QVERIFY(klass);
+    QVERIFY(klass->hasVirtualDestructor());
+    klass = AbstractMetaClass::findClass(classes, QLatin1String("Subject"));
+    QVERIFY(klass);
+    QVERIFY(klass->hasVirtualDestructor());
+}
+
 void TestDtorInformation::testClassWithVirtualDtorIsPolymorphic()
 {
     const char* cppCode ="class Control { public: virtual ~Control() {} }; class Subject { protected: virtual ~Subject() {} };";
diff --git a/sources/shiboken2/ApiExtractor/tests/testdtorinformation.h b/sources/shiboken2/ApiExtractor/tests/testdtorinformation.h
index 068ef4952..0a57dd8d1 100644
--- a/sources/shiboken2/ApiExtractor/tests/testdtorinformation.h
+++ b/sources/shiboken2/ApiExtractor/tests/testdtorinformation.h
@@ -40,6 +40,7 @@ private slots:
     void testDtorIsPrivate();
     void testDtorIsProtected();
     void testDtorIsVirtual();
+    void testDtorFromBaseIsVirtual();
     void testClassWithVirtualDtorIsPolymorphic();
 };
 
-- 
cgit v1.2.3