shiboken: Refactor metatype comparison

Remove unused code left over from previous changes
and add flags for matching value and const-ref as
equivalent.
Rename a few functions for improved clarity.

Change-Id: Ifac1414e4977643b18d31dfc63a8e4a5f89a2ddc
Reviewed-by: Cristian Maureira-Fredes <cristian.maureira-fredes@qt.io>

3 files changed, 50 insertions, 42 deletions

diff --git a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
index 0ecf2dce4..4566ed3bc 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetabuilder.cpp
@@ -1284,27 +1284,37 @@ void AbstractMetaBuilderPrivate::fixReturnTypeOfConversionOperator(AbstractMetaF
     metaFunction->replaceType(metaType);
 }
 
-static bool _compareAbstractMetaTypes(const AbstractMetaType *type, const AbstractMetaType *other)
+static bool _compareAbstractMetaTypes(const AbstractMetaType *type,
+                                      const AbstractMetaType *other,
+                                      AbstractMetaType::ComparisonFlags flags = {})
 {
     return (type != nullptr) == (other != nullptr)
-        && (type == nullptr || *type == *other);
+        && (type == nullptr || type->compare(*other, flags));
 }
 
-static bool _compareAbstractMetaFunctions(const AbstractMetaFunction *func, const AbstractMetaFunction *other)
+static bool _compareAbstractMetaFunctions(const AbstractMetaFunction *func,
+                                          const AbstractMetaFunction *other,
+                                          AbstractMetaType::ComparisonFlags argumentFlags = {})
 {
     if (!func && !other)
         return true;
     if (!func || !other)
         return false;
-    if (func->arguments().count() != other->arguments().count()
+    if (func->name() != other->name())
+        return false;
+    const int argumentsCount = func->arguments().count();
+    if (argumentsCount != other->arguments().count()
         || func->isConstant() != other->isConstant()
         || func->isStatic() != other->isStatic()
         || !_compareAbstractMetaTypes(func->type(), other->type())) {
         return false;
     }
-    for (int i = 0; i < func->arguments().count(); ++i) {
-        if (!_compareAbstractMetaTypes(func->arguments().at(i)->type(), other->arguments().at(i)->type()))
+    for (int i = 0; i < argumentsCount; ++i) {
+        if (!_compareAbstractMetaTypes(func->arguments().at(i)->type(),
+                                       other->arguments().at(i)->type(),
+                                       argumentFlags)) {
             return false;
+        }
     }
     return true;
 }
@@ -1330,29 +1340,6 @@ AbstractMetaFunctionList AbstractMetaBuilderPrivate::classFunctionList(const Sco
     return result;
 }
 
-// For template classes, entries with more specific types may exist from out-of-
-// line definitions. If there is a declaration which matches it after fixing
-// the parameters, remove it as duplicate. For example:
-// template class<T> Vector { public:
-//     Vector(const Vector &rhs);
-// };
-// template class<T>
-// Vector<T>::Vector(const Vector<T>&) {} // More specific, remove declaration.
-
-class DuplicatingFunctionPredicate : public std::unary_function<bool, const AbstractMetaFunction *> {
-public:
-    explicit DuplicatingFunctionPredicate(const AbstractMetaFunction *f) : m_function(f) {}
-
-    bool operator()(const AbstractMetaFunction *rhs) const
-    {
-        return rhs != m_function && rhs->name() == m_function->name()
-            && _compareAbstractMetaFunctions(m_function, rhs);
-    }
-
-private:
-    const AbstractMetaFunction *m_function;
-};
-
 void AbstractMetaBuilderPrivate::traverseFunctions(ScopeModelItem scopeItem,
                                                    AbstractMetaClass *metaClass)
 {
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
index 324dd8192..5ae671d87 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.cpp
@@ -246,11 +246,16 @@ AbstractMetaTypeCList AbstractMetaType::nestedArrayTypes() const
     return result;
 }
 
-bool AbstractMetaType::isConstRef() const
+bool AbstractMetaType::passByConstRef() const
 {
     return isConstant() && m_referenceType == LValueReference && indirections() == 0;
 }
 
+bool AbstractMetaType::passByValue() const
+{
+    return m_referenceType == NoReference && indirections() == 0;
+}
+
 QString AbstractMetaType::cppSignature() const
 {
     if (m_cachedCppSignature.isEmpty())
@@ -272,7 +277,7 @@ AbstractMetaType::TypeUsagePattern AbstractMetaType::determineUsagePattern() con
     if (m_typeEntry->isTemplateArgument() || m_referenceType == RValueReference)
         return InvalidPattern;
 
-    if (m_typeEntry->isPrimitive() && (actualIndirections() == 0 || isConstRef()))
+    if (m_typeEntry->isPrimitive() && (actualIndirections() == 0 || passByConstRef()))
         return PrimitivePattern;
 
     if (m_typeEntry->isVoid())
@@ -281,7 +286,7 @@ AbstractMetaType::TypeUsagePattern AbstractMetaType::determineUsagePattern() con
     if (m_typeEntry->isVarargs())
         return VarargsPattern;
 
-    if (m_typeEntry->isEnum() && (actualIndirections() == 0 || isConstRef()))
+    if (m_typeEntry->isEnum() && (actualIndirections() == 0 || passByConstRef()))
         return EnumPattern;
 
     if (m_typeEntry->isObject()) {
@@ -296,7 +301,7 @@ AbstractMetaType::TypeUsagePattern AbstractMetaType::determineUsagePattern() con
     if (m_typeEntry->isSmartPointer() && indirections() == 0)
         return SmartPointerPattern;
 
-    if (m_typeEntry->isFlags() && (actualIndirections() == 0 || isConstRef()))
+    if (m_typeEntry->isFlags() && (actualIndirections() == 0 || passByConstRef()))
         return FlagsPattern;
 
     if (m_typeEntry->isArray())
@@ -342,21 +347,29 @@ bool AbstractMetaType::hasTemplateChildren() const
     return false;
 }
 
-bool AbstractMetaType::equals(const AbstractMetaType &rhs) const
+bool AbstractMetaType::compare(const AbstractMetaType &rhs, ComparisonFlags flags) const
 {
-    if (m_typeEntry != rhs.m_typeEntry || m_constant != rhs.m_constant
-        || m_referenceType != rhs.m_referenceType
+    if (m_typeEntry != rhs.m_typeEntry
         || m_indirections != rhs.m_indirections
         || m_instantiations.size() != rhs.m_instantiations.size()
         || m_arrayElementCount != rhs.m_arrayElementCount) {
         return false;
     }
+
+    if (m_constant != rhs.m_constant || m_referenceType != rhs.m_referenceType) {
+        if (!flags.testFlag(ConstRefMatchesValue)
+            || !(passByValue() || passByConstRef())
+            || !(rhs.passByValue() || rhs.passByConstRef())) {
+            return false;
+        }
+    }
+
     if ((m_arrayElementType != nullptr) != (rhs.m_arrayElementType != nullptr)
-        || (m_arrayElementType != nullptr && !m_arrayElementType->equals(*rhs.m_arrayElementType))) {
+        || (m_arrayElementType != nullptr && !m_arrayElementType->compare(*rhs.m_arrayElementType, flags))) {
         return false;
     }
     for (int i = 0, size = m_instantiations.size(); i < size; ++i) {
-        if (!m_instantiations.at(i)->equals(*rhs.m_instantiations.at(i)))
+        if (!m_instantiations.at(i)->compare(*rhs.m_instantiations.at(i), flags))
                 return false;
     }
     return true;
diff --git a/sources/shiboken2/ApiExtractor/abstractmetalang.h b/sources/shiboken2/ApiExtractor/abstractmetalang.h
index 2a620a972..2ae1b6d21 100644
--- a/sources/shiboken2/ApiExtractor/abstractmetalang.h
+++ b/sources/shiboken2/ApiExtractor/abstractmetalang.h
@@ -302,6 +302,11 @@ public:
     };
     Q_ENUM(TypeUsagePattern)
 
+    enum ComparisonFlag {
+        ConstRefMatchesValue = 0x1
+    };
+    Q_DECLARE_FLAGS(ComparisonFlags, ComparisonFlag);
+
     AbstractMetaType();
     ~AbstractMetaType();
 
@@ -428,7 +433,8 @@ public:
     bool isVolatile() const { return m_volatile; }
     void setVolatile(bool v) { m_volatile = v; }
 
-    bool isConstRef() const;
+    bool passByConstRef() const;
+    bool passByValue() const;
 
     ReferenceType referenceType() const { return m_referenceType; }
     void setReferenceType(ReferenceType ref) { m_referenceType = ref; }
@@ -527,7 +533,7 @@ public:
 
     bool hasTemplateChildren() const;
 
-    bool equals(const AbstractMetaType &rhs) const;
+    bool compare(const AbstractMetaType &rhs, ComparisonFlags = {}) const;
 
 private:
     TypeUsagePattern determineUsagePattern() const;
@@ -558,10 +564,12 @@ private:
     Q_DISABLE_COPY(AbstractMetaType)
 };
 
+Q_DECLARE_OPERATORS_FOR_FLAGS(AbstractMetaType::ComparisonFlags);
+
 inline bool operator==(const AbstractMetaType &t1, const AbstractMetaType &t2)
-{ return t1.equals(t2); }
+{ return t1.compare(t2); }
 inline bool operator!=(const AbstractMetaType &t1, const AbstractMetaType &t2)
-{ return !t1.equals(t2); }
+{ return !t1.compare(t2); }
 
 #ifndef QT_NO_DEBUG_STREAM
 QDebug operator<<(QDebug d, const AbstractMetaType *at);