Refactor and improve added function resolving

Add a public function to create an AddedFunction::TypeInfo from a
signature string. Change translateType to use this to do full recursive
resolution on template arguments. Create a new helper function to do
more intelligent parsing of template type name signatures, and use this
in translateType.

Taken together, this means that we now recognize both multiple arguments
to template types of added functions, as well as template arguments that
are themselves template types. We couldn't do this before due to the use
of a regular expression to attempt to separate the template type name
from its arguments. The test cases are updated to test these new
features.

Note, however, that it is now a fatal error if the template argument
type cannot be determined. (Before we would just drop it and treat the
type as a non-container.) However, 1) this should not be happening in a
well formed type system, and 2) it's questionable if things wouldn't
have fallen apart anyway from treating the presumed container as
something else.

Change-Id: I23c3a7f7e311e046642d951d6ad2236571fda6b8
Reviewed-by: John Cummings <jcummings2@users.sf.net>

5 files changed, 151 insertions, 14 deletions

diff --git a/ApiExtractor/abstractmetabuilder.cpp b/ApiExtractor/abstractmetabuilder.cpp
index 8bac23c..0b94414 100644
--- a/ApiExtractor/abstractmetabuilder.cpp
+++ b/ApiExtractor/abstractmetabuilder.cpp
@@ -55,6 +55,55 @@ static QString stripTemplateArgs(const QString &name)
     return pos < 0 ? name : name.left(pos);
 }
 
+static QStringList parseTemplateType(const QString& name) {
+    int n = name.indexOf('<');
+    if (n <= 0) {
+        // If name starts with '<' or contains an unmatched (i.e. any) '>', we
+        // reject it
+        if (n == 0 || name.count(">"))
+            return QStringList();
+        // Doesn't look like a template instantiation; just return the name
+        return QStringList() << name;
+    }
+
+    // Split the type name into the template name and template arguments; the
+    // part before the opening '<' is the template name
+    //
+    // Example:
+    //   "foo<A, bar<B, C>, D>" -> ( "foo", "A", "bar<B, C>", "D" )
+    QStringList result;
+    result << name.left(n).trimmed();
+
+    // Extract template arguments
+    int i, depth = 1;
+    const int l = name.length();
+    for (i = n + 1; i < l; ++i) {
+        // Consume balanced '<'/'>' within a single argument so that we won't
+        // split on ',' as part of a single argument which is itself a
+        // multi-argument template type
+        if (name[i] == '<') {
+            ++depth;
+        } else if (name[i] == '>') {
+            if (--depth == 0)
+                break;
+        } else if (name[i] == ',' && depth == 1) {
+            // Encountered ',' in template argument list that is not within
+            // another template name; add current argument to result and start
+            // working on the next argument
+            result << name.mid(n + 1, i - n - 1).trimmed();
+            n = i;
+        }
+    }
+    if (i >= l) // arg list not closed
+        return QStringList();
+    if (i + 1 < l) // arg list closed before end of name
+        return QStringList();
+
+    // Add final argument and return result
+    result << name.mid(n + 1, i - n - 1).trimmed();
+    return result;
+}
+
 AbstractMetaBuilder::AbstractMetaBuilder() : m_currentClass(0), m_logDirectory(QString('.')+QDir::separator())
 {
 }
@@ -2002,15 +2051,14 @@ AbstractMetaType* AbstractMetaBuilder::translateType(double vr, const AddedFunct
 
     // test if the type is a template, like a container
     bool isTemplate = false;
-    QString templateArg;
-    if (!type) {
-        QRegExp r("(.*)<(.*)>$");
-        if (r.indexIn(typeInfo.name) != -1) {
-            templateArg = r.cap(2);
-            if (templateArg.contains(','))
-                ReportHandler::warning("add-function tag doesn't support container types with more than one argument or template arguments.");
-            else
-                isTemplate = (type = typeDb->findContainerType(r.cap(1)));
+    QStringList templateArgs;
+    if (!type && typeInfo.name.contains('<')) {
+        const QStringList& parsedType = parseTemplateType(typeInfo.name);
+        if (parsedType.isEmpty()) {
+            ReportHandler::warning(QString("Template type parsing failed for '%1'").arg(typeInfo.name));
+        } else {
+            templateArgs = parsedType.mid(1);
+            isTemplate = (type = typeDb->findContainerType(parsedType[0]));
         }
     }
 
@@ -2042,13 +2090,11 @@ AbstractMetaType* AbstractMetaBuilder::translateType(double vr, const AddedFunct
     metaType->setReference(typeInfo.isReference);
     metaType->setConstant(typeInfo.isConstant);
     if (isTemplate) {
-        type = typeDb->findType(templateArg);
-        if (type) {
-            AbstractMetaType* metaArgType = createMetaType();
-            metaArgType->setTypeEntry(type);
+        foreach (const QString& templateArg, templateArgs) {
+            AbstractMetaType* metaArgType = translateType(vr, AddedFunction::TypeInfo::fromSignature(templateArg));
             metaType->addInstantiation(metaArgType);
-            metaType->setTypeUsagePattern(AbstractMetaType::ContainerPattern);
         }
+        metaType->setTypeUsagePattern(AbstractMetaType::ContainerPattern);
     }
 
     return metaType;
diff --git a/ApiExtractor/typesystem.cpp b/ApiExtractor/typesystem.cpp
index 9a99ce5..9925a39 100644
--- a/ApiExtractor/typesystem.cpp
+++ b/ApiExtractor/typesystem.cpp
@@ -2202,6 +2202,11 @@ AddedFunction::AddedFunction(QString signature, QString returnType, double vr) :
     }
 }
 
+AddedFunction::TypeInfo AddedFunction::TypeInfo::fromSignature(const QString& signature)
+{
+    return parseType(signature);
+}
+
 QString ComplexTypeEntry::targetLangApiName() const
 {
     return strings_jobject;
diff --git a/ApiExtractor/typesystem.h b/ApiExtractor/typesystem.h
index 98f5cb7..0549672 100644
--- a/ApiExtractor/typesystem.h
+++ b/ApiExtractor/typesystem.h
@@ -473,6 +473,8 @@ struct AddedFunction
     */
     struct TypeInfo {
         TypeInfo() : isConstant(false), indirections(0), isReference(false) {}
+        static TypeInfo fromSignature(const QString& signature);
+
         QString name;
         bool isConstant;
         int indirections;
diff --git a/tests/samplebinding/addedfunction_with_container_args_test.py b/tests/samplebinding/addedfunction_with_container_args_test.py
new file mode 100644
index 0000000..85971eb
--- /dev/null
+++ b/tests/samplebinding/addedfunction_with_container_args_test.py
@@ -0,0 +1,48 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+#
+# Copyright 2013 Kitware, Inc.
+#
+# This file is part of the Shiboken Python Bindings Generator project.
+#
+# Copyright (C) 2013 Digia Plc and/or its subsidiary(-ies).
+#
+# Contact: PySide team <contact@pyside.org>
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public License
+# version 2.1 as published by the Free Software Foundation. Please
+# review the following information to ensure the GNU Lesser General
+# Public License version 2.1 requirements will be met:
+# http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+# #
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+# 02110-1301 USA
+
+'''Test cases for added functions with nested and multi-argument container types.'''
+
+import unittest
+from sample import sum2d, sumproduct
+
+class TestAddedFunctionsWithContainerArgs(unittest.TestCase):
+    '''Tests added functions with nested and multi-argument container types.'''
+
+    def testNestedContainerType(self):
+        '''Test added function with single-argument containers.'''
+        values = [[1,2],[3,4,5],[6]]
+        self.assertEqual(sum2d(values), 21)
+
+    def testMultiArgContainerType(self):
+        '''Test added function with a two-argument container.'''
+        values = [(1,2),(3,4),(5,6)]
+        self.assertEqual(sumproduct(values), 44)
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tests/samplebinding/typesystem_sample.xml b/tests/samplebinding/typesystem_sample.xml
index 4e3a924..824697f 100644
--- a/tests/samplebinding/typesystem_sample.xml
+++ b/tests/samplebinding/typesystem_sample.xml
@@ -1513,6 +1513,42 @@
         </modify-function>
     </function>
 
+    <!-- Tests add-function for nested template types -->
+    <add-function signature="sum2d(std::list&lt;std::list&lt;int&gt; &gt;)" return-type="int">
+        <inject-code class="target" position="beginning">
+            typedef std::list&lt;int&gt; Inner;
+            typedef std::list&lt;Inner&gt; Outer;
+
+            int result = 0;
+
+            Outer::const_iterator oiter, oend = %1.end();
+            for (oiter = %1.begin(); oiter != oend; ++oiter) {
+                const Inner&amp; inner = *oiter;
+                Inner::const_iterator iiter, iend = inner.end();
+                for (iiter = inner.begin(); iiter != iend; ++iiter)
+                    result += *iiter;
+            }
+
+            %PYARG_0 = %CONVERTTOPYTHON[int](result);
+        </inject-code>
+    </add-function>
+
+    <!-- Tests add-function for nested template types -->
+    <add-function signature="sumproduct(std::list&lt;std::pair&lt;int, int&gt; &gt;)" return-type="int">
+        <inject-code class="target" position="beginning">
+            typedef std::pair&lt;int, int&gt; Pair;
+            typedef std::list&lt;Pair&gt; List;
+
+            int result = 0;
+
+            List::const_iterator iter, end = %1.end();
+            for (iter = %1.begin(); iter != end; ++iter)
+                result += iter->first * iter->second;
+
+            %PYARG_0 = %CONVERTTOPYTHON[int](result);
+        </inject-code>
+    </add-function>
+
 
     <value-type name="InjectCode">
         <!--