shiboken6: Refactor wrapper function generation checking

Besides the actual bindings, shiboken needs to generate functions into
the wrapper class. The checks for this were spread over various
functions, causing for example the override cache array being too
large (due to inconsistent checks for the QMetaObject special
functions and other special cases).

Centralize this in a function returning flags indicating
the wrapper usage.

Change-Id: I48f6d6dd46d673f916c9ead0dd18c13b04d75855
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>
(cherry picked from commit 9ba60057a22693242dab3b3edc6ba1240561e6da)

5 files changed, 110 insertions, 85 deletions

diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
index 2db5cb63e..3f1c72988 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
@@ -581,17 +581,13 @@ void CppGenerator::generateClass(TextStream &s, const GeneratorContext &classCon
             s << "}\n\n";
         }
 
-        const auto &funcs = filterFunctions(metaClass);
         int maxOverrides = 0;
         writeCacheResetNative(s, classContext);
-        for (const auto &func : funcs) {
-            const bool notAbstract = !func->isAbstract();
-            if ((func->isPrivate() && notAbstract && !func->isVisibilityModifiedToPrivate())
-                || (func->isModifiedRemoved() && notAbstract))
-                continue;
-            if (func->functionType() == AbstractMetaFunction::ConstructorFunction && !func->isUserAdded())
+        for (const auto &func : metaClass->functions()) {
+            const auto generation = functionGeneration(func);
+            if (generation.testFlag(FunctionGenerationFlag::WrapperConstructor))
                 writeConstructorNative(s, classContext, func);
-            else if (shouldWriteVirtualMethodNative(func))
+            else if (generation.testFlag(FunctionGenerationFlag::VirtualMethod))
                 writeVirtualMethodNative(s, func, maxOverrides++);
         }
 
@@ -1124,12 +1120,6 @@ void CppGenerator::writeVirtualMethodNative(TextStream &s,
                                             const AbstractMetaFunctionCPtr &func,
                                             int cacheIndex) const
 {
-    // skip metaObject function, this will be written manually ahead
-    if (usePySideExtensions() && func->ownerClass() && func->ownerClass()->isQObject() &&
-        ((func->name() == u"metaObject"_s)
-            || (func->name() == u"qt_metacall")))
-        return;
-
     const TypeEntry *retType = func->type().typeEntry();
     const QString funcName = func->isOperatorOverload()
         ? pythonOperatorFunctionName(func) : func->definitionNames().constFirst();
diff --git a/sources/shiboken6/generator/shiboken/headergenerator.cpp b/sources/shiboken6/generator/shiboken/headergenerator.cpp
index 9465bce8c..01bb5d578 100644
--- a/sources/shiboken6/generator/shiboken/headergenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/headergenerator.cpp
@@ -138,15 +138,13 @@ void HeaderGenerator::generateClass(TextStream &s, const GeneratorContext &class
             s << '\n';
         }
 
-        const auto &funcs = filterFunctions(metaClass);
         int maxOverrides = 0;
-        for (const auto &func : funcs) {
-            if (!func->attributes().testFlag(AbstractMetaFunction::FinalCppMethod)) {
-                writeFunction(s, func);
-                // PYSIDE-803: Build a boolean cache for unused overrides.
-                if (shouldWriteVirtualMethodNative(func))
-                    maxOverrides++;
-            }
+        for (const auto &func : metaClass->functions()) {
+            const auto generation = functionGeneration(func);
+            writeFunction(s, func, generation);
+            // PYSIDE-803: Build a boolean cache for unused overrides.
+            if (generation.testFlag(FunctionGenerationFlag::VirtualMethod))
+                maxOverrides++;
         }
         if (!maxOverrides)
             maxOverrides = 1;
@@ -247,43 +245,34 @@ void HeaderGenerator::writeMemberFunctionWrapper(TextStream &s,
     s << "); }\n";
 }
 
-void HeaderGenerator::writeFunction(TextStream &s, const AbstractMetaFunctionCPtr &func)
+void HeaderGenerator::writeFunction(TextStream &s, const AbstractMetaFunctionCPtr &func,
+                                    FunctionGeneration generation)
 {
 
     // do not write copy ctors here.
-    if (!func->isPrivate() && func->functionType() == AbstractMetaFunction::CopyConstructorFunction) {
+    if (generation.testFlag(FunctionGenerationFlag::WrapperSpecialCopyConstructor)) {
         writeCopyCtor(s, func->ownerClass());
         return;
     }
-    if (func->isUserAdded())
-        return;
 
-    if (avoidProtectedHack() && func->isProtected() && !func->isConstructor()
-        && !func->isOperatorOverload()) {
+    if (generation.testFlag(FunctionGenerationFlag::ProtectedWrapper))
         writeMemberFunctionWrapper(s, func, u"_protected"_s);
-    }
-
-    // pure virtual functions need a default implementation
-    const bool notAbstract = !func->isAbstract();
-    if ((func->isPrivate() && notAbstract && !func->isVisibilityModifiedToPrivate())
-        || (func->isModifiedRemoved() && notAbstract))
-        return;
 
-    if (avoidProtectedHack() && func->ownerClass()->hasPrivateDestructor()
-        && (func->isAbstract() || func->isVirtual()))
-        return;
-
-    if (func->functionType() == AbstractMetaFunction::ConstructorFunction) {
+    if (generation.testFlag(FunctionGenerationFlag::WrapperConstructor)) {
         Options option =  func->hasSignatureModifications()
             ? Generator::OriginalTypeDescription : Generator::NoOption;
         s << functionSignature(func, {}, {}, option) << ";\n";
         return;
     }
 
-    if (func->isAbstract() || func->isVirtual()) {
+    const bool isVirtual = generation.testFlag(FunctionGenerationFlag::VirtualMethod);
+    if (isVirtual || generation.testFlag(FunctionGenerationFlag::QMetaObjectMethod)) {
         s << functionSignature(func, {}, {}, Generator::OriginalTypeDescription)
             << " override;\n";
-        // Check if this method hide other methods in base classes
+    }
+
+    // Check if this method hide other methods in base classes
+    if (isVirtual) {
         for (const auto &f : func->ownerClass()->functions()) {
             if (f != func
                 && !f->isConstructor()
diff --git a/sources/shiboken6/generator/shiboken/headergenerator.h b/sources/shiboken6/generator/shiboken/headergenerator.h
index 3771d45cf..3adaa247b 100644
--- a/sources/shiboken6/generator/shiboken/headergenerator.h
+++ b/sources/shiboken6/generator/shiboken/headergenerator.h
@@ -54,7 +54,8 @@ protected:
 
 private:
     void writeCopyCtor(TextStream &s, const AbstractMetaClass *metaClass) const;
-    void writeFunction(TextStream &s, const AbstractMetaFunctionCPtr &func);
+    void writeFunction(TextStream &s, const AbstractMetaFunctionCPtr &func,
+                       FunctionGeneration generation);
     void writeSbkTypeFunction(TextStream &s, const AbstractMetaEnum &cppEnum) const;
     static void writeSbkTypeFunction(TextStream &s, const AbstractMetaClass *cppClass) ;
     static void writeSbkTypeFunction(TextStream &s, const AbstractMetaType &metaType) ;
diff --git a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
index bac9ed6b5..268cb7c25 100644
--- a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
@@ -199,13 +199,70 @@ bool ShibokenGenerator::shouldGenerateCppWrapper(const AbstractMetaClass *metaCl
             && wrapper.testFlag(AbstractMetaClass::CppProtectedHackWrapper));
 }
 
-bool ShibokenGenerator::shouldWriteVirtualMethodNative(const AbstractMetaFunctionCPtr &func) const
+ShibokenGenerator::FunctionGeneration
+    ShibokenGenerator::functionGeneration(const AbstractMetaFunctionCPtr &func) const
 {
-    // PYSIDE-803: Extracted this because it is used multiple times.
-    const AbstractMetaClass *metaClass = func->ownerClass();
-    return (!avoidProtectedHack() || !metaClass->hasPrivateDestructor())
-            && ((func->isVirtual() || func->isAbstract())
-                 && !func->attributes().testFlag(AbstractMetaFunction::FinalCppMethod));
+    FunctionGeneration result;
+
+    const auto functionType = func->functionType();
+    switch (functionType) {
+    case AbstractMetaFunction::ConversionOperator:
+    case AbstractMetaFunction::AssignmentOperatorFunction:
+    case AbstractMetaFunction::MoveAssignmentOperatorFunction:
+    case AbstractMetaFunction::DestructorFunction:
+    case AbstractMetaFunction::SignalFunction:
+    case AbstractMetaFunction::GetAttroFunction:
+    case AbstractMetaFunction::SetAttroFunction:
+        return result;
+    default:
+        if (func->isUserAdded() || func->usesRValueReferences())
+            return result;
+        break;
+    }
+
+    const bool notModifiedRemoved = !func->isModifiedRemoved();
+    const bool isPrivate = func->isPrivate() && !func->isVisibilityModifiedToPrivate();
+    switch (functionType) {
+    case AbstractMetaFunction::ConstructorFunction:
+        if (!isPrivate && notModifiedRemoved)
+             result.setFlag(FunctionGenerationFlag::WrapperConstructor);
+        return result;
+    case AbstractMetaFunction::CopyConstructorFunction:
+        if (!isPrivate && notModifiedRemoved)
+            result.setFlag(FunctionGenerationFlag::WrapperSpecialCopyConstructor);
+        return result;
+    case AbstractMetaFunction::NormalFunction:
+    case AbstractMetaFunction::SlotFunction:
+        if (avoidProtectedHack() && func->isProtected())
+            result.setFlag(FunctionGenerationFlag::ProtectedWrapper);
+        break;
+    default:
+        break;
+    }
+
+    // Check on virtuals (including operators).
+    const bool isAbstract = func->isAbstract();
+    if (!(isAbstract || func->isVirtual())
+        || func->attributes().testFlag(AbstractMetaFunction::FinalCppMethod)) {
+        return result;
+    }
+
+    // MetaObject virtuals only need to be declared; CppGenerator creates a
+    // special implementation.
+    if (functionType == AbstractMetaFunction::NormalFunction
+        && usePySideExtensions() && func->ownerClass()->isQObject()) {
+        const QString &name = func->name();
+        if (name == u"metaObject"_s || name == u"qt_metacall") {
+            result.setFlag(FunctionGenerationFlag::QMetaObjectMethod);
+            return result;
+        }
+    }
+
+    // Pure virtual functions need a default implementation even if private.
+    if (isAbstract || (notModifiedRemoved && !isPrivate))
+        result.setFlag(FunctionGenerationFlag::VirtualMethod);
+
+    return result;
 }
 
 AbstractMetaFunctionCList ShibokenGenerator::implicitConversions(const TypeEntry *t) const
@@ -1152,38 +1209,6 @@ void ShibokenGenerator::writeUnusedVariableCast(TextStream &s, const QString &va
     s << "SBK_UNUSED(" << variableName<< ")\n";
 }
 
-static bool filterFunction(const AbstractMetaFunctionCPtr &func, bool avoidProtectedHack)
-{
-    switch (func->functionType()) {
-    case AbstractMetaFunction::DestructorFunction:
-    case AbstractMetaFunction::SignalFunction:
-    case AbstractMetaFunction::GetAttroFunction:
-    case AbstractMetaFunction::SetAttroFunction:
-        return false;
-    default:
-        break;
-    }
-    if (func->usesRValueReferences())
-        return false;
-    if (func->isModifiedRemoved() && !func->isAbstract()
-        && (!avoidProtectedHack || !func->isProtected())) {
-        return false;
-    }
-    return true;
-}
-
-AbstractMetaFunctionCList ShibokenGenerator::filterFunctions(const AbstractMetaClass *metaClass) const
-{
-    AbstractMetaFunctionCList result;
-    const AbstractMetaFunctionCList &funcs = metaClass->functions();
-    result.reserve(funcs.size());
-    for (const auto &func : funcs) {
-        if (filterFunction(func, avoidProtectedHack()))
-            result.append(func);
-    }
-    return result;
-}
-
 ShibokenGenerator::ExtendedConverterData ShibokenGenerator::getExtendedConverters() const
 {
     ExtendedConverterData extConvs;
diff --git a/sources/shiboken6/generator/shiboken/shibokengenerator.h b/sources/shiboken6/generator/shiboken/shibokengenerator.h
index e1f717644..14829d08f 100644
--- a/sources/shiboken6/generator/shiboken/shibokengenerator.h
+++ b/sources/shiboken6/generator/shiboken/shibokengenerator.h
@@ -52,6 +52,27 @@ QT_FORWARD_DECLARE_CLASS(TextStream)
 class ShibokenGenerator : public Generator
 {
 public:
+    /// Besides the actual bindings (see AbstractMetaFunction::generateBinding(),
+    /// some functions need to be generated into the wrapper class
+    /// (virtual method/avoid protected hack expose).
+    enum class FunctionGenerationFlag
+    {
+        None = 0x0,
+        /// Virtual method overridable in Python
+        VirtualMethod = 0x1,
+        /// Special QObject virtuals
+        QMetaObjectMethod = 0x2,
+        /// Needs a protected wrapper for avoidProtectedHack()
+        /// public "foo_protected()" calling "foo()"
+        ProtectedWrapper = 0x4,        //
+        /// Pass through constructor
+        WrapperConstructor = 0x8,
+        /// Generate a special copy constructor
+        /// "FooBar_Wrapper(const Foo&)" for constructing a wrapper from a value
+        WrapperSpecialCopyConstructor = 0x10
+    };
+    Q_DECLARE_FLAGS(FunctionGeneration, FunctionGenerationFlag);
+
     enum class AttroCheckFlag
     {
         None                   = 0x0,
@@ -176,8 +197,8 @@ protected:
     /// Verifies if the class should have a C++ wrapper generated for it, instead of only a Python wrapper.
     bool shouldGenerateCppWrapper(const AbstractMetaClass *metaClass) const;
 
-    /// Condition to call WriteVirtualMethodNative. Was extracted because also used to count these calls.
-    bool shouldWriteVirtualMethodNative(const AbstractMetaFunctionCPtr &func) const;
+    /// Returns which functions need to be generated into the wrapper class
+    FunctionGeneration functionGeneration(const AbstractMetaFunctionCPtr &func) const;
 
     // Return a list of implicit conversions if generation is enabled.
     AbstractMetaFunctionCList implicitConversions(const TypeEntry *t) const;
@@ -297,8 +318,6 @@ protected:
 
     static void writeUnusedVariableCast(TextStream &s, const QString &variableName);
 
-    AbstractMetaFunctionCList filterFunctions(const AbstractMetaClass *metaClass) const;
-
     // All data about extended converters: the type entries of the target type, and a
     // list of AbstractMetaClasses accepted as argument for the conversion.
     using ExtendedConverterData = QHash<const TypeEntry *, AbstractMetaClassCList>;
@@ -446,6 +465,7 @@ private:
     static const TypeSystemConverterRegExps &typeSystemConvRegExps();
 };
 
+Q_DECLARE_OPERATORS_FOR_FLAGS(ShibokenGenerator::FunctionGeneration);
 Q_DECLARE_OPERATORS_FOR_FLAGS(ShibokenGenerator::AttroCheck);
 
 extern const QString CPP_ARG;