shiboken: Refactor code involved in inheriting template classes

- Add a helper template to conveniently search for a MetaClass by name
  and convenience function to search in lists.
- Remove parameter bool *ok from
  AbstractMetaBuilderPrivate::inheritTemplateType() and check on returned
  pointer instead.
- In the inheritTemplate*() functions, use QScopedPointer to ensure
  the result is deleted on failure.
- Search for a shadowing function first before creating the copy
  in inheritTemplate().

Task-number: PYSIDE-725
Change-Id: I3183087fb42b22a2189c17b94eaafdb4c1151f0e
Reviewed-by: Christian Tismer <tismer@stackless.com>

4 files changed, 81 insertions, 70 deletions

diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
index b0c212f89..cbe33ae22 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
@@ -2752,52 +2752,48 @@ bool AbstractMetaBuilderPrivate::ancestorHasPrivateCopyConstructor(const Abstrac
     return false;
 }
 
-AbstractMetaType* AbstractMetaBuilderPrivate::inheritTemplateType(const QVector<AbstractMetaType *> &templateTypes,
-                                                                  const AbstractMetaType *metaType,
-                                                                  bool *ok)
+AbstractMetaType *
+    AbstractMetaBuilderPrivate::inheritTemplateType(const AbstractMetaTypeList &templateTypes,
+                                                    const AbstractMetaType *metaType)
 {
-    if (ok)
-        *ok = true;
-    if (!metaType || (!metaType->typeEntry()->isTemplateArgument() && !metaType->hasInstantiations()))
-        return metaType ? metaType->copy() : 0;
+    Q_ASSERT(metaType);
+
+    QScopedPointer<AbstractMetaType> returned(metaType->copy());
+
+    if (!metaType->typeEntry()->isTemplateArgument() && !metaType->hasInstantiations())
+        return returned.take();
 
-    AbstractMetaType *returned = metaType->copy();
-    returned->setOriginalTemplateType(metaType->copy());
+    returned->setOriginalTemplateType(metaType);
 
     if (returned->typeEntry()->isTemplateArgument()) {
         const TemplateArgumentEntry* tae = static_cast<const TemplateArgumentEntry*>(returned->typeEntry());
 
         // If the template is intantiated with void we special case this as rejecting the functions that use this
         // parameter from the instantiation.
-        if (templateTypes.size() <= tae->ordinal() || templateTypes.at(tae->ordinal())->typeEntry()->name() == QLatin1String("void")) {
-            if (ok)
-                *ok = false;
-            return 0;
-        }
+        const AbstractMetaType *templateType = templateTypes.value(tae->ordinal());
+        if (!templateType || templateType->typeEntry()->isVoid())
+            return nullptr;
 
         AbstractMetaType* t = returned->copy();
-        t->setTypeEntry(templateTypes.at(tae->ordinal())->typeEntry());
-        t->setIndirections(templateTypes.at(tae->ordinal())->indirections() + t->indirections() ? 1 : 0);
+        t->setTypeEntry(templateType->typeEntry());
+        t->setIndirections(templateType->indirections() + t->indirections() ? 1 : 0);
         t->decideUsagePattern();
 
-        delete returned;
-        returned = inheritTemplateType(templateTypes, t, ok);
-        if (ok && !(*ok))
-            return 0;
+        return inheritTemplateType(templateTypes, t);
     }
 
     if (returned->hasInstantiations()) {
         AbstractMetaTypeList instantiations = returned->instantiations();
         for (int i = 0; i < instantiations.count(); ++i) {
-            AbstractMetaType *type = instantiations[i];
-            instantiations[i] = inheritTemplateType(templateTypes, type, ok);
-            if (ok && !(*ok))
-                return 0;
+            instantiations[i] =
+                inheritTemplateType(templateTypes, instantiations.at(i));
+            if (!instantiations.at(i))
+                return nullptr;
         }
         returned->setInstantiations(instantiations, true);
     }
 
-    return returned;
+    return returned.take();
 }
 
 bool AbstractMetaBuilderPrivate::inheritTemplate(AbstractMetaClass *subclass,
@@ -2850,38 +2846,38 @@ bool AbstractMetaBuilderPrivate::inheritTemplate(AbstractMetaClass *subclass,
         }
     }
 
-    AbstractMetaFunctionList funcs = subclass->functions();
+    const AbstractMetaFunctionList &subclassFuncs = subclass->functions();
     const AbstractMetaFunctionList &templateClassFunctions = templateClass->functions();
     for (const AbstractMetaFunction *function : templateClassFunctions) {
-        if (function->isModifiedRemoved(TypeSystem::All))
+        // If the function is modified or the instantiation has an equally named
+        // function we have shadowing, so we need to skip it.
+        if (function->isModifiedRemoved(TypeSystem::All)
+            || AbstractMetaFunction::find(subclassFuncs, function->name()) != nullptr) {
             continue;
+        }
 
-        AbstractMetaFunction *f = function->copy();
+        QScopedPointer<AbstractMetaFunction> f(function->copy());
         f->setArguments(AbstractMetaArgumentList());
 
-        bool ok = true;
-        AbstractMetaType *ftype = function->type();
-        f->replaceType(inheritTemplateType(templateTypes, ftype, &ok));
-        if (!ok) {
-            delete f;
-            continue;
+        if (function->type()) { // Non-void
+            AbstractMetaType *returnType = inheritTemplateType(templateTypes, function->type());
+            if (!returnType)
+                continue;
+            f->replaceType(returnType);
         }
 
         const AbstractMetaArgumentList &arguments = function->arguments();
         for (AbstractMetaArgument *argument : arguments) {
-            AbstractMetaType* atype = argument->type();
-
-            AbstractMetaArgument *arg = argument->copy();
-            arg->replaceType(inheritTemplateType(templateTypes, atype, &ok));
-            if (!ok)
+            AbstractMetaType *argType = inheritTemplateType(templateTypes, argument->type());
+            if (!argType)
                 break;
+            AbstractMetaArgument *arg = argument->copy();
+            arg->replaceType(argType);
             f->addArgument(arg);
         }
 
-        if (!ok) {
-            delete f;
+        if (f->arguments().size() < function->arguments().size())
             continue;
-        }
 
         // There is no base class in the target language to inherit from here, so
         // the template instantiation is the class that implements the function.
@@ -2892,23 +2888,11 @@ bool AbstractMetaBuilderPrivate::inheritTemplate(AbstractMetaClass *subclass,
         // on the inherited functions.
         f->setDeclaringClass(subclass);
 
-
-        if (f->isConstructor() && subclass->isTypeDef()) {
+        if (f->isConstructor()) {
+            if (!subclass->isTypeDef())
+                continue;
             f->setName(subclass->name());
             f->setOriginalName(subclass->name());
-        } else if (f->isConstructor()) {
-            delete f;
-            continue;
-        }
-
-        // if the instantiation has a function named the same as an existing
-        // function we have shadowing so we need to skip it.
-        const bool found =
-            std::any_of(funcs.cbegin(), funcs.cend(),
-                        [f] (const AbstractMetaFunction *needle) { return needle->name() == f->name(); });
-        if (found) {
-            delete f;
-            continue;
         }
 
         ComplexTypeEntry* te = subclass->typeEntry();
@@ -2935,7 +2919,7 @@ bool AbstractMetaBuilderPrivate::inheritTemplate(AbstractMetaClass *subclass,
             te->addFunctionModification(mod);
         }
 
-        subclass->addFunction(f);
+        subclass->addFunction(f.take());
     }
 
     subclass->setTemplateBaseClass(templateClass);
diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h b/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h
index 7f4d222a3..9fcad26ad 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder_p.h
@@ -142,9 +142,8 @@ public:
     bool inheritTemplate(AbstractMetaClass *subclass,
                          const AbstractMetaClass *templateClass,
                          const TypeInfo &info);
-    AbstractMetaType *inheritTemplateType(const QVector<AbstractMetaType *> &templateTypes,
-                                          const AbstractMetaType *metaType,
-                                          bool *ok = Q_NULLPTR);
+    AbstractMetaType *inheritTemplateType(const AbstractMetaTypeList &templateTypes,
+                                          const AbstractMetaType *metaType);
 
     bool isQObject(const FileModelItem &dom, const QString &qualifiedName);
     bool isEnum(const FileModelItem &dom, const QStringList &qualifiedName);
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
index 69a1390dc..e9069277a 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
@@ -57,6 +57,16 @@ QDebug operator<<(QDebug d, const AbstractMetaAttributes *aa)
 }
 #endif // !QT_NO_DEBUG_STREAM
 
+template <class MetaClass>
+MetaClass *findByName(QVector<MetaClass *> haystack, QStringView needle)
+{
+    for (MetaClass *c : haystack) {
+        if (c->name() == needle)
+            return c;
+    }
+    return nullptr;
+}
+
 /*******************************************************************************
  * AbstractMetaVariable
  */
@@ -1117,6 +1127,13 @@ bool function_sorter(AbstractMetaFunction *a, AbstractMetaFunction *b)
     return a->signature() < b->signature();
 }
 
+AbstractMetaFunction *
+AbstractMetaFunction::find(const AbstractMetaFunctionList &haystack,
+                           const QString &needle)
+{
+    return findByName(haystack, needle);
+}
+
 #ifndef QT_NO_DEBUG_STREAM
 static inline void formatMetaFunctionBrief(QDebug &d, const AbstractMetaFunction *af)
 {
@@ -1513,11 +1530,7 @@ bool AbstractMetaClass::hasFunction(const QString &str) const
 
 const AbstractMetaFunction* AbstractMetaClass::findFunction(const QString& functionName) const
 {
-    for (const AbstractMetaFunction *f : m_functions) {
-        if (f->name() == functionName)
-            return f;
-    }
-    return 0;
+    return AbstractMetaFunction::find(m_functions, functionName);
 }
 
 bool AbstractMetaClass::hasProtectedFunctions() const
@@ -1618,6 +1631,11 @@ AbstractMetaField *AbstractMetaField::copy() const
     return returned;
 }
 
+AbstractMetaField *AbstractMetaField::find(const AbstractMetaFieldList &haystack,
+                                           const QString &needle)
+{
+    return findByName(haystack, needle);
+}
 /*******************************************************************************
  * Indicates that this field has a modification that removes it
  */
@@ -2013,13 +2031,15 @@ void AbstractMetaClass::setInterfaces(const AbstractMetaClassList &interfaces)
     }
 }
 
+AbstractMetaField *AbstractMetaClass::findField(const QString &name) const
+{
+    return AbstractMetaField::find(m_fields, name);
+}
 
 AbstractMetaEnum *AbstractMetaClass::findEnum(const QString &enumName)
 {
-    for (AbstractMetaEnum *e : qAsConst(m_enums)) {
-        if (e->name() == enumName)
-            return e;
-    }
+    if (AbstractMetaEnum *e = findByName(m_enums, enumName))
+        return e;
 
     if (typeEntry()->designatedInterface())
         return extractInterface()->findEnum(enumName);
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.h b/sources/shiboken2/ApiExtractor/abstractmetalang.h
index f55f61eb4..54a3549d0 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.h
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.h
@@ -727,6 +727,9 @@ public:
 
     AbstractMetaField *copy() const;
 
+    static AbstractMetaField *
+        find(const AbstractMetaFieldList &haystack, const QString &needle);
+
 private:
     mutable AbstractMetaFunction *m_getter = nullptr;
     mutable AbstractMetaFunction *m_setter = nullptr;
@@ -1079,6 +1082,9 @@ public:
 
     bool isCallOperator() const;
 
+    static AbstractMetaFunction *
+        find(const AbstractMetaFunctionList &haystack, const QString &needle);
+
 #ifndef QT_NO_DEBUG_STREAM
     void formatDebugVerbose(QDebug &d) const;
 #endif
@@ -1409,6 +1415,8 @@ public:
         m_fields << field;
     }
 
+    AbstractMetaField *findField(const QString &name) const;
+
     AbstractMetaEnumList enums() const
     {
         return m_enums;