shiboken6: Handle hidden methods/"using" correctly

In C++, declaring a non-override method in a class hides all methods
of the same name from the base class unless they are made visible by a
"using Base::name" specification.

Shiboken did not observe this rule; base class methods were added
nevertheless, causing problems with code snippets.

In addition, there were several places where the recursion
for the inherited base class methods was done.

Move the collection of inherited base class methods into
ShibokenGenerator::getFunctionGroups() and implement proper handling
of using declarations.

This function then returns the authoritative list of functions to be
generated.

Remove a few cases from the test.

[ChangeLog][shiboken6] The handling of hidden base class
member functions and using declarations has been fixed.

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

5 files changed, 86 insertions, 79 deletions

diff --git a/sources/shiboken6/ApiExtractor/abstractmetalang.cpp b/sources/shiboken6/ApiExtractor/abstractmetalang.cpp
index c2d9bfcf6..91da71cb9 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetalang.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetalang.cpp
@@ -1087,6 +1087,14 @@ bool AbstractMetaClass::isUsingMember(const AbstractMetaClass *c,
     return d->isUsingMember(c, memberName, minimumAccess);
 }
 
+bool AbstractMetaClass::hasUsingMemberFor(const QString &memberName) const
+{
+    return std::any_of(d->m_usingMembers.cbegin(), d->m_usingMembers.cend(),
+                       [&memberName](const UsingMember &um) {
+                          return um.memberName == memberName;
+                       });
+}
+
 /* Goes through the list of functions and returns a list of all
    functions matching all of the criteria in \a query.
  */
diff --git a/sources/shiboken6/ApiExtractor/abstractmetalang.h b/sources/shiboken6/ApiExtractor/abstractmetalang.h
index 39d5a4a8e..f08ed2039 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetalang.h
+++ b/sources/shiboken6/ApiExtractor/abstractmetalang.h
@@ -146,6 +146,7 @@ public:
     void addUsingMember(const UsingMember &um);
     bool isUsingMember(const AbstractMetaClass *c, const QString &memberName,
                        Access minimumAccess) const;
+    bool hasUsingMemberFor(const QString &memberName) const;
 
     AbstractMetaFunctionCList queryFunctionsByName(const QString &name) const;
     static bool queryFunction(const AbstractMetaFunction *f, FunctionQueryOptions query);
diff --git a/sources/shiboken6/generator/shiboken/cppgenerator.cpp b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
index 557324ceb..941603ea9 100644
--- a/sources/shiboken6/generator/shiboken/cppgenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/cppgenerator.cpp
@@ -534,32 +534,7 @@ void CppGenerator::generateClass(TextStream &s, const GeneratorContext &classCon
         << "extern \"C\" {\n";
     const auto &functionGroups = getFunctionGroups(metaClass);
     for (auto it = functionGroups.cbegin(), end = functionGroups.cend(); it != end; ++it) {
-        AbstractMetaFunctionCList overloads;
-        QSet<QString> seenSignatures;
-        bool staticEncountered = false;
-        for (const auto &func : it.value()) {
-            if (func->ownerClass() == func->implementingClass()
-                && func->generateBinding()) {
-                // PYSIDE-331: Inheritance works correctly when there are disjoint functions.
-                // But when a function is both in a class and inherited in a subclass,
-                // then we need to search through all subclasses and collect the new signatures.
-                overloads << getFunctionAndInheritedOverloads(func, &seenSignatures);
-                if (func->isStatic())
-                    staticEncountered = true;
-            }
-        }
-        // PYSIDE-886: If the method does not have any static overloads declared
-        // in the class in question, remove all inherited static methods as setting
-        // METH_STATIC in that case can cause crashes for the instance methods.
-        // Manifested as crash when calling QPlainTextEdit::find() (clash with
-        // static QWidget::find(WId)).
-        if (!staticEncountered) {
-            for (qsizetype i = overloads.size() - 1; i >= 0; --i) {
-                if (overloads.at(i)->isStatic())
-                    overloads.removeAt(i);
-            }
-        }
-
+        const AbstractMetaFunctionCList &overloads = it.value();
         if (overloads.isEmpty())
             continue;
 
diff --git a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
index c443b0208..6672fb64c 100644
--- a/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
+++ b/sources/shiboken6/generator/shiboken/shibokengenerator.cpp
@@ -33,6 +33,8 @@
 #include <abstractmetafield.h>
 #include <abstractmetafunction.h>
 #include <abstractmetalang.h>
+#include <abstractmetalang_helpers.h>
+#include <usingmember.h>
 #include <exception.h>
 #include <messages.h>
 #include <modifications.h>
@@ -1840,7 +1842,7 @@ void ShibokenGenerator::writeCodeSnips(TextStream &s,
         bool isProtected = func->isProtected();
         auto owner = func->ownerClass();
         if (!isProtected && func->isUserAdded() && owner != nullptr) {
-            const auto &funcs = getMethodOverloads(owner, func->name());
+            const auto &funcs = getFunctionGroups(owner).value(func->name());
             isProtected = std::any_of(funcs.cbegin(), funcs.cend(),
                                       [](const AbstractMetaFunctionCPtr &f) {
                                           return f->isProtected();
@@ -2288,10 +2290,12 @@ ShibokenGenerator::FunctionGroups ShibokenGenerator::getFunctionGroupsImpl(const
 
     FunctionGroups results;
     for (const auto &func : lst) {
-        if (isGroupable(func)) {
+        if (isGroupable(func)
+            && func->ownerClass() == func->implementingClass()
+            && func->generateBinding()) {
             auto it = results.find(func->name());
             if (it == results.end()) {
-                results.insert(func->name(), AbstractMetaFunctionCList(1, func));
+                it = results.insert(func->name(), AbstractMetaFunctionCList(1, func));
             } else {
                 // If there are virtuals methods in the mix (PYSIDE-570,
                 // QFileSystemModel::index(QString,int) and
@@ -2303,60 +2307,85 @@ ShibokenGenerator::FunctionGroups ShibokenGenerator::getFunctionGroupsImpl(const
                 else
                     it.value().append(func);
             }
+            getInheritedOverloads(scope, &it.value());
         }
     }
     return results;
 }
 
-AbstractMetaFunctionCList
-    ShibokenGenerator::getInheritedOverloads(const AbstractMetaFunctionCPtr &func, QSet<QString> *seen)
-{
-    AbstractMetaFunctionCList results;
-    AbstractMetaClass *basis;
-    if (func->ownerClass() && (basis = func->ownerClass()->baseClass())) {
-        for (; basis; basis = basis->baseClass()) {
-            const auto inFunctions = basis->findFunctions(func->name());
-            for (const auto &inFunc : inFunctions) {
-                if (inFunc->generateBinding()
-                    && !seen->contains(inFunc->minimalSignature())) {
-                    seen->insert(inFunc->minimalSignature());
-                    AbstractMetaFunction *newFunc = inFunc->copy();
-                    newFunc->setImplementingClass(func->implementingClass());
-                    results << AbstractMetaFunctionCPtr(newFunc);
-                }
-            }
-        }
-    }
-    return results;
-}
-
-AbstractMetaFunctionCList
-    ShibokenGenerator::getFunctionAndInheritedOverloads(const AbstractMetaFunctionCPtr &func,
-                                                        QSet<QString> *seen)
+static bool hidesBaseClassFunctions(const AbstractMetaFunctionCPtr &f)
 {
-    AbstractMetaFunctionCList results;
-    seen->insert(func->minimalSignature());
-    results << func << getInheritedOverloads(func, seen);
-    return results;
+    return 0 == (f->attributes()
+                 & (AbstractMetaFunction::OverriddenCppMethod | AbstractMetaFunction::FinalCppMethod));
 }
 
-AbstractMetaFunctionCList ShibokenGenerator::getMethodOverloads(const AbstractMetaClass *scope,
-                                                                const QString &functionName) const
+void ShibokenGenerator::getInheritedOverloads(const AbstractMetaClass *scope,
+                                             AbstractMetaFunctionCList *overloads)
 {
-    Q_ASSERT(scope);
-    const auto &lst = scope->functions();
+    if (overloads->isEmpty() || scope->isNamespace() || scope->baseClasses().isEmpty())
+        return;
+
+    // PYSIDE-331: look also into base classes. Check for any non-overriding
+    // function hiding the base class functions.
+    const bool hideBaseClassFunctions =
+        std::any_of(overloads->cbegin(), overloads->cend(), hidesBaseClassFunctions);
+
+    const QString &functionName = overloads->constFirst()->name();
+    const bool hasUsingDeclarations = scope->hasUsingMemberFor(functionName);
+    if (hideBaseClassFunctions && !hasUsingDeclarations)
+        return; // No base function is visible
 
-    AbstractMetaFunctionCList results;
+    // Collect base candidates by name and signature
+    bool staticEncountered = false;
     QSet<QString> seenSignatures;
-    for (const auto &func : qAsConst(lst)) {
-        if (func->name() != functionName)
-            continue;
-        if (isGroupable(func)) {
-            // PYSIDE-331: look also into base classes.
-            results << getFunctionAndInheritedOverloads(func, &seenSignatures);
+    for (const auto &func : *overloads) {
+        seenSignatures.insert(func->minimalSignature());
+        staticEncountered |= func->isStatic();
+    }
+
+    AbstractMetaFunctionCList baseCandidates;
+
+    auto basePredicate = [&functionName, &seenSignatures, &baseCandidates](const AbstractMetaClass *b) {
+        for (const auto &f : b->functions()) {
+            if (f->generateBinding() && f->name() == functionName) {
+                const QString signature = f->minimalSignature();
+                if (!seenSignatures.contains(signature)) {
+                    seenSignatures.insert(signature);
+                    baseCandidates.append(f);
+                }
+            }
         }
+        return false; // Keep going
+    };
+
+    for (const auto *baseClass : scope->baseClasses())
+        recurseClassHierarchy(baseClass, basePredicate);
+
+    // Remove the ones that are not made visible with using declarations
+    if (hideBaseClassFunctions && hasUsingDeclarations) {
+        const auto pred =  [scope](const AbstractMetaFunctionCPtr &f) {
+            return !scope->isUsingMember(f->ownerClass(), f->name(), f->access());
+        };
+        auto end = std::remove_if(baseCandidates.begin(), baseCandidates.end(), pred);
+        baseCandidates.erase(end, baseCandidates.end());
+    }
+
+    // PYSIDE-886: If the method does not have any static overloads declared
+    // in the class in question, remove all inherited static methods as setting
+    // METH_STATIC in that case can cause crashes for the instance methods.
+    // Manifested as crash when calling QPlainTextEdit::find() (clash with
+    // static QWidget::find(WId)).
+    if (!staticEncountered) {
+        auto end =  std::remove_if(baseCandidates.begin(), baseCandidates.end(),
+                                   [](const AbstractMetaFunctionCPtr &f) { return f->isStatic(); });
+         baseCandidates.erase(end, baseCandidates.end());
+    }
+
+    for (const auto &baseCandidate : baseCandidates) {
+        AbstractMetaFunction *newFunc = baseCandidate->copy();
+        newFunc->setImplementingClass(scope);
+        overloads->append(AbstractMetaFunctionCPtr(newFunc));
     }
-    return results;
 }
 
 Generator::OptionDescriptions ShibokenGenerator::options() const
diff --git a/sources/shiboken6/generator/shiboken/shibokengenerator.h b/sources/shiboken6/generator/shiboken/shibokengenerator.h
index fd492842e..ad80e14a3 100644
--- a/sources/shiboken6/generator/shiboken/shibokengenerator.h
+++ b/sources/shiboken6/generator/shiboken/shibokengenerator.h
@@ -417,16 +417,10 @@ private:
      *   \param func the metafunction to be searched in subclasses.
      *   \param seen the function's minimal signatures already seen.
      */
-    static AbstractMetaFunctionCList getInheritedOverloads(const AbstractMetaFunctionCPtr & func,
-                                                          QSet<QString> *seen);
+    static void getInheritedOverloads(const AbstractMetaClass *scope,
+                                      AbstractMetaFunctionCList *overloads);
+
 
-    /**
-     *   Returns all overloads for a function named \p functionName.
-     *   \param scope scope used to search for overloads.
-     *   \param functionName the function name.
-     */
-    AbstractMetaFunctionCList getMethodOverloads(const AbstractMetaClass *scope,
-                                                const QString &functionName) const;
     /**
      *   Write a function argument in the C++ in the text stream \p s.
      *   This function just call \code s << argumentString(); \endcode