shiboken: Add allow-thread attribute to type system and class entries

In the typesystem parser, add the allow-thread attribute to root and
complex type entry.

Rewrite the handling of allow-thread (cached) in AbstractMetaFunction
similar to the exception handling (store the modification in
AbstractMetaFunction and go down the class hierarchy if it is
unspecified).

Change-Id: I00e6e2ab25208fda63ec20522814cbfccbb8c42d
Fixes: PYSIDE-931
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>

8 files changed, 97 insertions, 35 deletions

diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
index 41ae5d7f1..37ff3b72c 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
@@ -2058,6 +2058,8 @@ AbstractMetaFunction *AbstractMetaBuilderPrivate::traverseFunction(const Functio
     for (const FunctionModification &mod : functionMods) {
         if (mod.exceptionHandling() != TypeSystem::ExceptionHandling::Unspecified)
             metaFunction->setExceptionHandlingModification(mod.exceptionHandling());
+        else if (mod.allowThread() != TypeSystem::AllowThread::Unspecified)
+            metaFunction->setAllowThreadModification(mod.allowThread());
     }
 
     // Find the correct default values
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
index 660bbdff1..a10a15b08 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
@@ -550,6 +550,7 @@ AbstractMetaFunction *AbstractMetaFunction::copy() const
         cpy->setType(type()->copy());
     cpy->setConstant(isConstant());
     cpy->setExceptionSpecification(m_exceptionSpecification);
+    cpy->setAllowThreadModification(m_allowThreadModification);
     cpy->setExceptionHandlingModification(m_exceptionHandlingModification);
 
     for (AbstractMetaArgument *arg : m_arguments)
@@ -773,28 +774,40 @@ bool AbstractMetaFunction::autoDetectAllowThread() const
     return !maybeGetter;
 }
 
-bool AbstractMetaFunction::allowThread() const
+static inline TypeSystem::AllowThread allowThreadMod(const AbstractMetaClass *klass)
 {
-    using AllowThread = TypeSystem::AllowThread;
+    return klass->typeEntry()->allowThread();
+}
 
-    if (m_cachedAllowThread < 0) {
-        AllowThread allowThread = AllowThread::Auto;
-        // Find a modification that specifies allowThread
-        const FunctionModificationList &modifications = this->modifications(declaringClass());
-        for (const FunctionModification &modification : modifications) {
-            if (modification.allowThread() != AllowThread::Unspecified) {
-                allowThread = modification.allowThread();
-                break;
-            }
-        }
+static inline bool hasAllowThreadMod(const AbstractMetaClass *klass)
+{
+    return allowThreadMod(klass) != TypeSystem::AllowThread::Unspecified;
+}
 
-        m_cachedAllowThread = allowThread == AllowThread::Allow
-            || (allowThread == AllowThread::Auto && autoDetectAllowThread()) ? 1 : 0;
+bool AbstractMetaFunction::allowThread() const
+{
+    auto allowThreadModification = m_allowThreadModification;
+    // If there is no modification on the function, check for a base class.
+    if (m_class && allowThreadModification == TypeSystem::AllowThread::Unspecified) {
+        if (auto base = recurseClassHierarchy(m_class, hasAllowThreadMod))
+            allowThreadModification = allowThreadMod(base);
+    }
 
-        if (m_cachedAllowThread == 0)
-            qCDebug(lcShiboken).noquote() << msgDisallowThread(this);
+    bool result = true;
+    switch (allowThreadModification) {
+    case TypeSystem::AllowThread::Disallow:
+        result = false;
+        break;
+    case TypeSystem::AllowThread::Allow:
+        break;
+    case TypeSystem::AllowThread::Auto:
+    case TypeSystem::AllowThread::Unspecified:
+        result = autoDetectAllowThread();
+        break;
     }
-    return m_cachedAllowThread > 0;
+    if (!result)
+        qCDebug(lcShiboken).noquote() << msgDisallowThread(this);
+    return result;
 }
 
 TypeSystem::Ownership AbstractMetaFunction::ownership(const AbstractMetaClass *cls, TypeSystem::Language language, int key) const
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.h b/sources/shiboken2/ApiExtractor/abstractmetalang.h
index 56b36b638..074adbe00 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.h
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.h
@@ -1096,6 +1096,8 @@ public:
         find(const AbstractMetaFunctionList &haystack, const QString &needle);
 
     // for the meta builder only
+    void setAllowThreadModification(TypeSystem::AllowThread am)
+    { m_allowThreadModification = am; }
     void setExceptionHandlingModification(TypeSystem::ExceptionHandling em)
     { m_exceptionHandlingModification = em;  }
 
@@ -1126,8 +1128,8 @@ private:
     uint m_explicit                 : 1;
     uint m_pointerOperator          : 1;
     uint m_isCallOperator           : 1;
-    mutable int m_cachedAllowThread = -1;
     ExceptionSpecification m_exceptionSpecification = ExceptionSpecification::Unknown;
+    TypeSystem::AllowThread m_allowThreadModification = TypeSystem::AllowThread::Unspecified;
     TypeSystem::ExceptionHandling m_exceptionHandlingModification = TypeSystem::ExceptionHandling::Unspecified;
 };
 
diff --git a/sources/shiboken2/ApiExtractor/doc/typesystem_specifying_types.rst b/sources/shiboken2/ApiExtractor/doc/typesystem_specifying_types.rst
index c3180ae88..ce66f77fe 100644
--- a/sources/shiboken2/ApiExtractor/doc/typesystem_specifying_types.rst
+++ b/sources/shiboken2/ApiExtractor/doc/typesystem_specifying_types.rst
@@ -11,7 +11,7 @@ typesystem
 
     .. code-block:: xml
 
-        <typesystem package="..." default-superclass="..." exception-handling="...">
+        <typesystem package="..." default-superclass="..." allow-thread="..." exception-handling="...">
         </typesystem>
 
     The **package** attribute is a string describing the package to be used,
@@ -19,8 +19,9 @@ typesystem
     The *optional* **default-superclass** attribute is the canonical C++ base class
     name of all objects, e.g., "object".
 
-    The *optional* **exception-handling** attribute specifies the default exception
-    handling mode of all objects (see :ref:`modify-function`).
+    The *optional* attributes **allow-thread** and **exception-handling**
+    specify the default handling for the corresponding function modification
+    (see :ref:`modify-function`).
 
 load-typesystem
 ^^^^^^^^^^^^^^^
@@ -219,6 +220,7 @@ value-type
         <typesystem>
             <value-type  name="..." since="..."
              copyable="yes | no"
+             allow-thread="..."
              exception-handling="..."
              hash-function="..."
              stream="yes | no"
@@ -247,8 +249,9 @@ value-type
     The **revision** attribute can be used to specify a revision for each type, easing the
     production of ABI compatible bindings.
 
-    The *optional* **exception-handling** attribute specifies the default exception
-    handling mode of all functions (see :ref:`modify-function`).
+    The *optional* attributes **allow-thread** and **exception-handling**
+    specify the default handling for the corresponding function modification
+    (see :ref:`modify-function`).
 
 .. _object-type:
 
@@ -265,6 +268,7 @@ object-type
             <object-type name="..."
              since="..."
              copyable="yes | no"
+             allow-thread="..."
              exception-handling="..."
              hash-function="..."
              stream="yes | no"
@@ -286,8 +290,9 @@ object-type
     The **revision** attribute can be used to specify a revision for each type, easing the
     production of ABI compatible bindings.
 
-    The *optional* **exception-handling** attribute specifies the default exception
-    handling mode of all functions (see :ref:`modify-function`).
+    The *optional* attributes **allow-thread** and **exception-handling**
+    specify the default handling for the corresponding function modification
+    (see :ref:`modify-function`).
 
 interface-type
 ^^^^^^^^^^^^^^
diff --git a/sources/shiboken2/ApiExtractor/tests/testmodifyfunction.cpp b/sources/shiboken2/ApiExtractor/tests/testmodifyfunction.cpp
index 88e0c500d..922f1c23f 100644
--- a/sources/shiboken2/ApiExtractor/tests/testmodifyfunction.cpp
+++ b/sources/shiboken2/ApiExtractor/tests/testmodifyfunction.cpp
@@ -222,6 +222,8 @@ void TestModifyFunction::testWithApiVersion()
     QVERIFY(func->ownership(func->ownerClass(), TypeSystem::TargetLangCode, 0) != TypeSystem::CppOwnership);
 }
 
+// Modifications on class/typesystem level are tested below
+// in testScopedModifications().
 void TestModifyFunction::testAllowThread()
 {
     const char cppCode[] =R"CPP(\
@@ -315,6 +317,8 @@ void TestModifyFunction::testGlobalFunctionModification()
     QCOMPARE(arg->defaultValueExpression(), QLatin1String("A()"));
 }
 
+// Tests modifications of exception handling and allow-thread
+// on various levels.
 void TestModifyFunction::testScopedModifications_data()
 {
     QTest::addColumn<QByteArray>("cppCode");
@@ -322,6 +326,7 @@ void TestModifyFunction::testScopedModifications_data()
     QTest::addColumn<bool>("expectedGenerateUnspecified");
     QTest::addColumn<bool>("expectedGenerateNonThrowing");
     QTest::addColumn<bool>("expectedGenerateThrowing");
+    QTest::addColumn<bool>("expectedAllowThread");
 
     const QByteArray cppCode = R"CPP(
 struct Base {
@@ -343,7 +348,8 @@ struct A : public Base {
     <object-type name='Base'/>
     <object-type name='A'/>
 </typesystem>)XML")
-         << false << false << false;
+         << false << false << false // exception
+         << true; // allowthread
 
     // Modify one function
     QTest::newRow("modify-function1")
@@ -356,7 +362,8 @@ struct A : public Base {
         <modify-function signature='throwing()' exception-handling='auto-on'/>
     </object-type>
 </typesystem>)XML")
-         << false << false << true;
+         << false << false << true // exception
+         << true; // allowthread
 
     // Flip defaults by modifying functions
     QTest::newRow("modify-function2")
@@ -370,18 +377,20 @@ struct A : public Base {
         <modify-function signature='throwing()' exception-handling='off'/>
     </object-type>
 </typesystem>)XML")
-         << true << false << false;
+         << true << false << false // exception
+         << true; // allowthread
 
     // Activate on type system level
     QTest::newRow("typesystem-on")
         << cppCode
         << QByteArray(R"XML(
-<typesystem package='Foo' exception-handling='auto-on'>
+<typesystem package='Foo' exception-handling='auto-on' allow-thread='no'>
     <primitive-type name='int'/>
     <object-type name='Base'/>
     <object-type name='A'/>
 </typesystem>)XML")
-         << true << false << true;
+         << true << false << true // exception
+         << false; // allowthread
 
     // Activate on class level
     QTest::newRow("class-on")
@@ -390,9 +399,10 @@ struct A : public Base {
 <typesystem package='Foo'>
     <primitive-type name='int'/>
     <object-type name='Base'/>
-    <object-type name='A' exception-handling='auto-on'/>
+    <object-type name='A' exception-handling='auto-on' allow-thread='no'/>
 </typesystem>)XML")
-         << true << false << true;
+         << true << false << true // exception
+         << false; // allowthread
 
     // Activate on base class level
     QTest::newRow("baseclass-on")
@@ -400,10 +410,11 @@ struct A : public Base {
         << QByteArray(R"XML(
 <typesystem package='Foo'>
     <primitive-type name='int'/>
-    <object-type name='Base' exception-handling='auto-on'/>
+    <object-type name='Base' exception-handling='auto-on' allow-thread='no'/>
     <object-type name='A'/>
 </typesystem>)XML")
-         << true << false << true;
+         << true << false << true // exception
+         << false; // allowthread
 
     // Override value on class level
     QTest::newRow("override-class-on")
@@ -416,7 +427,8 @@ struct A : public Base {
         <modify-function signature='throwing()' exception-handling='no'/>
     </object-type>
 </typesystem>)XML")
-         << true << false << false;
+         << true << false << false // exception
+         << true; // allowthread
 }
 
 void TestModifyFunction::testScopedModifications()
@@ -426,6 +438,7 @@ void TestModifyFunction::testScopedModifications()
     QFETCH(bool, expectedGenerateUnspecified);
     QFETCH(bool, expectedGenerateNonThrowing);
     QFETCH(bool, expectedGenerateThrowing);
+    QFETCH(bool, expectedAllowThread);
 
     QScopedPointer<AbstractMetaBuilder> builder(TestUtil::parse(cppCode.constData(), xmlCode.constData(), false));
     QVERIFY(!builder.isNull());
@@ -437,6 +450,7 @@ void TestModifyFunction::testScopedModifications()
     QVERIFY(f);
     QCOMPARE(f->exceptionSpecification(), ExceptionSpecification::Unknown);
     QCOMPARE(f->generateExceptionHandling(), expectedGenerateUnspecified);
+    QCOMPARE(f->allowThread(), expectedAllowThread);
 
     f = classA->findFunction(QStringLiteral("nonThrowing"));
     QVERIFY(f);
diff --git a/sources/shiboken2/ApiExtractor/typesystem.cpp b/sources/shiboken2/ApiExtractor/typesystem.cpp
index ff4f74d8c..204253777 100644
--- a/sources/shiboken2/ApiExtractor/typesystem.cpp
+++ b/sources/shiboken2/ApiExtractor/typesystem.cpp
@@ -1281,6 +1281,7 @@ void Handler::applyComplexTypeAttributes(const QXmlStreamReader &reader,
     bool generate = true;
     ctype->setCopyable(ComplexTypeEntry::Unknown);
     auto exceptionHandling = m_exceptionHandling;
+    auto allowThread = m_allowThread;
 
     QString package = m_defaultPackage;
     for (int i = attributes->size() - 1; i >= 0; --i) {
@@ -1316,6 +1317,15 @@ void Handler::applyComplexTypeAttributes(const QXmlStreamReader &reader,
                 qCWarning(lcShiboken, "%s",
                           qPrintable(msgInvalidAttributeValue(attribute)));
             }
+        } else if (name == allowThreadAttribute()) {
+            const auto attribute = attributes->takeAt(i);
+            const auto v = allowThreadFromAttribute(attribute.value());
+            if (v != TypeSystem::AllowThread::Unspecified) {
+                allowThread = v;
+            } else {
+                qCWarning(lcShiboken, "%s",
+                          qPrintable(msgInvalidAttributeValue(attribute)));
+            }
         } else if (name == QLatin1String("held-type")) {
             qCWarning(lcShiboken, "%s",
                       qPrintable(msgUnimplementedAttributeWarning(reader, name)));
@@ -1337,6 +1347,8 @@ void Handler::applyComplexTypeAttributes(const QXmlStreamReader &reader,
 
     if (exceptionHandling != TypeSystem::ExceptionHandling::Unspecified)
          ctype->setExceptionHandling(exceptionHandling);
+    if (allowThread != TypeSystem::AllowThread::Unspecified)
+        ctype->setAllowThread(allowThread);
 
     // The generator code relies on container's package being empty.
     if (ctype->type() != TypeEntry::ContainerType)
@@ -1483,6 +1495,15 @@ TypeSystemTypeEntry *Handler::parseRootElement(const QXmlStreamReader &,
                 qCWarning(lcShiboken, "%s",
                           qPrintable(msgInvalidAttributeValue(attribute)));
             }
+        } else if (name == allowThreadAttribute()) {
+            const auto attribute = attributes->takeAt(i);
+            const auto v = allowThreadFromAttribute(attribute.value());
+            if (v != TypeSystem::AllowThread::Unspecified) {
+                m_allowThread = v;
+            } else {
+                qCWarning(lcShiboken, "%s",
+                          qPrintable(msgInvalidAttributeValue(attribute)));
+            }
         }
     }
 
diff --git a/sources/shiboken2/ApiExtractor/typesystem.h b/sources/shiboken2/ApiExtractor/typesystem.h
index b0144923a..36a75c599 100644
--- a/sources/shiboken2/ApiExtractor/typesystem.h
+++ b/sources/shiboken2/ApiExtractor/typesystem.h
@@ -1395,6 +1395,9 @@ public:
     TypeSystem::ExceptionHandling exceptionHandling() const { return m_exceptionHandling; }
     void setExceptionHandling(TypeSystem::ExceptionHandling e) { m_exceptionHandling = e; }
 
+    TypeSystem::AllowThread allowThread() const { return m_allowThread; }
+    void setAllowThread(TypeSystem::AllowThread allowThread) { m_allowThread = allowThread; }
+
     QString defaultConstructor() const;
     void setDefaultConstructor(const QString& defaultConstructor);
     bool hasDefaultConstructor() const;
@@ -1433,6 +1436,7 @@ private:
     const ComplexTypeEntry* m_baseContainerType = nullptr;
     // For class functions
     TypeSystem::ExceptionHandling m_exceptionHandling = TypeSystem::ExceptionHandling::Unspecified;
+    TypeSystem::AllowThread m_allowThread = TypeSystem::AllowThread::Unspecified;
 };
 
 class TypedefEntry : public ComplexTypeEntry
diff --git a/sources/shiboken2/ApiExtractor/typesystem_p.h b/sources/shiboken2/ApiExtractor/typesystem_p.h
index a617110d6..a119b2a97 100644
--- a/sources/shiboken2/ApiExtractor/typesystem_p.h
+++ b/sources/shiboken2/ApiExtractor/typesystem_p.h
@@ -246,6 +246,7 @@ private:
     QString m_defaultPackage;
     QString m_defaultSuperclass;
     TypeSystem::ExceptionHandling m_exceptionHandling = TypeSystem::ExceptionHandling::Unspecified;
+    TypeSystem::AllowThread m_allowThread = TypeSystem::AllowThread::Unspecified;
     QString m_error;
     const TypeEntry::CodeGeneration m_generate;