Fixed type system variable replacement for arguments removed with conversion rule.

Also added an array argument modification test, and moved
AutoArrayPointer from libpyside to libshiboken.

Reviewed by Hugo Parente <hugo.lima@openbossa.org>
Reviewed by Luciano Wolf <luciano.wolf@openbossa.org>

8 files changed, 87 insertions, 56 deletions

diff --git a/generator/cppgenerator.cpp b/generator/cppgenerator.cpp
index 827ec28e3..efbf6ef96 100644
--- a/generator/cppgenerator.cpp
+++ b/generator/cppgenerator.cpp
@@ -726,7 +726,7 @@ void CppGenerator::writeVirtualMethodNative(QTextStream&s, const AbstractMetaFun
             ac << INDENT;
             if (!func->conversionRule(TypeSystem::TargetLangCode, arg->argumentIndex() + 1).isEmpty()) {
                 // Has conversion rule.
-                ac << arg->name() << "_out";
+                ac << QString("%1"CONV_RULE_OUT_VAR_SUFFIX).arg(arg->name());
             } else {
                 QString argName = arg->name();
                 if (avoidProtectedHack()) {
@@ -2021,17 +2021,16 @@ void CppGenerator::writeMethodCall(QTextStream& s, const AbstractMetaFunction* f
             int removedArgs = 0;
             for (int i = 0; i < maxArgs + removedArgs; i++) {
                 const AbstractMetaArgument* arg = func->arguments().at(i);
+                bool hasConversionRule = !func->conversionRule(TypeSystem::NativeCode, arg->argumentIndex() + 1).isEmpty();
                 if (func->argumentRemoved(i + 1)) {
-
                     // If some argument with default value is removed from a
                     // method signature, the said value must be explicitly
                     // added to the method call.
                     removedArgs++;
 
                     // If have conversion rules I will use this for removed args
-                    bool hasConversionRule = !func->conversionRule(TypeSystem::NativeCode, arg->argumentIndex() + 1).isEmpty();
                     if (hasConversionRule) {
-                        userArgs << arg->name() + "_out";
+                        userArgs << QString("%1"CONV_RULE_OUT_VAR_SUFFIX).arg(arg->name());
                     } else {
                        if (arg->defaultValueExpression().isEmpty())
                            badModifications = true;
@@ -2040,14 +2039,9 @@ void CppGenerator::writeMethodCall(QTextStream& s, const AbstractMetaFunction* f
                     }
                 } else {
                     int idx = arg->argumentIndex() - removedArgs;
-                    QString argName;
-
-                    bool hasConversionRule = !func->conversionRule(TypeSystem::NativeCode, arg->argumentIndex() + 1).isEmpty();
-                    if (hasConversionRule) {
-                        argName = arg->name() + "_out";
-                    } else {
-                        argName = QString(CPP_ARG"%1").arg(idx);
-                    }
+                    QString argName = hasConversionRule
+                                      ? QString("%1"CONV_RULE_OUT_VAR_SUFFIX).arg(arg->name())
+                                      : QString(CPP_ARG"%1").arg(idx);
                     userArgs << argName;
                 }
             }
@@ -2073,7 +2067,7 @@ void CppGenerator::writeMethodCall(QTextStream& s, const AbstractMetaFunction* f
                 if (!arg->defaultValueExpression().isEmpty())
                     otherArgs.prepend(guessScopeForDefaultValue(func, arg));
                 else if (hasConversionRule)
-                    otherArgs.prepend(arg->name() + "_out");
+                    otherArgs.prepend(QString("%1"CONV_RULE_OUT_VAR_SUFFIX).arg(arg->name()));
                 else
                     badModifications = true;
             }
diff --git a/generator/shibokengenerator.cpp b/generator/shibokengenerator.cpp
index 05d374ea4..eacc5e54d 100644
--- a/generator/shibokengenerator.cpp
+++ b/generator/shibokengenerator.cpp
@@ -1081,7 +1081,7 @@ void ShibokenGenerator::writeArgumentNames(QTextStream &s,
             && (!func->conversionRule(TypeSystem::NativeCode, arguments.at(j)->argumentIndex() + 1).isEmpty()
                 || !func->conversionRule(TypeSystem::TargetLangCode, arguments.at(j)->argumentIndex() + 1).isEmpty())
             && !func->isConstructor()) {
-           s << "_out";
+           s << CONV_RULE_OUT_VAR_SUFFIX;
         }
 
         argCount++;
@@ -1206,20 +1206,31 @@ QMap<int, QString> ShibokenGenerator::getArgumentReplacement(const AbstractMetaF
                                                              const AbstractMetaArgument* lastArg)
 {
     QMap<int, QString> argReplacement;
+    TypeSystem::Language convLang = (language == TypeSystem::TargetLangCode)
+                                    ? TypeSystem::NativeCode : TypeSystem::TargetLangCode;
     int removed = 0;
     for (int i = 0; i < func->arguments().size(); ++i) {
         const AbstractMetaArgument* arg = func->arguments().at(i);
         QString argValue;
         if (language == TypeSystem::TargetLangCode) {
+            bool hasConversionRule = !func->conversionRule(convLang, i+1).isEmpty();
             bool argRemoved = func->argumentRemoved(i+1);
             removed = removed + (int) argRemoved;
             if (argRemoved || (lastArg && arg->argumentIndex() > lastArg->argumentIndex()))
                 argValue = arg->defaultValueExpression();
+
+            if (argRemoved && hasConversionRule && argValue.isEmpty())
+                argValue = QString("%1"CONV_RULE_OUT_VAR_SUFFIX).arg(arg->name());
+
             if (!argRemoved && argValue.isEmpty()) {
-                if (arg->type()->typeEntry()->isCustom())
-                    argValue = usePyArgs ? QString(PYTHON_ARGS"[%1]").arg(i - removed) : PYTHON_ARG;
-                else
-                    argValue = QString(CPP_ARG"%1").arg(i - removed);
+                int argPos = i - removed;
+                if (arg->type()->typeEntry()->isCustom()) {
+                    argValue = usePyArgs ? QString(PYTHON_ARGS"[%1]").arg(argPos) : PYTHON_ARG;
+                } else {
+                    argValue = hasConversionRule
+                               ? QString("%1"CONV_RULE_OUT_VAR_SUFFIX).arg(arg->name())
+                               : QString(CPP_ARG"%1").arg(argPos);
+                }
             }
         } else {
             argValue = arg->name();
diff --git a/generator/shibokengenerator.h b/generator/shibokengenerator.h
index 605b205e6..2c4dc888a 100644
--- a/generator/shibokengenerator.h
+++ b/generator/shibokengenerator.h
@@ -24,6 +24,7 @@
 #ifndef SHIBOKENGENERATOR_H
 #define SHIBOKENGENERATOR_H
 
+#define CONV_RULE_OUT_VAR_SUFFIX  "_out"
 #define CPP_ARG                   "cppArg"
 #define CPP_ARG0                  CPP_ARG"0"
 #define CPP_RETURN_VAR            "cppResult"
diff --git a/libshiboken/helper.h b/libshiboken/helper.h
index b5e0b624e..f179b2d7c 100644
--- a/libshiboken/helper.h
+++ b/libshiboken/helper.h
@@ -92,6 +92,20 @@ LIBSHIBOKEN_API bool sequenceToArgcArgv(PyObject* argList, int* argc, char*** ar
  */
 LIBSHIBOKEN_API int* sequenceToIntArray(PyObject* obj, bool zeroTerminated = false);
 
+/**
+ *  Creates and automatically deallocates C++ arrays.
+ */
+template<class T>
+class AutoArrayPointer
+{
+    public:
+        AutoArrayPointer(int size) { data = new T[size]; }
+        T& operator[](int pos) { return data[pos]; }
+        operator T*() const { return data; }
+        ~AutoArrayPointer() { delete[] data; }
+    private:
+        T* data;
+};
 
 /**
  * An utility function used to call PyErr_WarnEx with a formatted message.
@@ -101,4 +115,3 @@ LIBSHIBOKEN_API int warning(PyObject *category, int stacklevel, const char *form
 } // namespace Shiboken
 
 #endif // HELPER_H
-
diff --git a/tests/libsample/modifications.cpp b/tests/libsample/modifications.cpp
index 8d3539814..00a0cdf0c 100644
--- a/tests/libsample/modifications.cpp
+++ b/tests/libsample/modifications.cpp
@@ -88,3 +88,11 @@ Modifications::className()
     return "Modifications";
 }
 
+Point
+Modifications::sumPointArray(int arraySize, const Point pointArray[])
+{
+    Point point;
+    for (int i = 0; i < arraySize; ++i)
+        point = point + pointArray[i];
+    return point;
+}
diff --git a/tests/libsample/modifications.h b/tests/libsample/modifications.h
index 639d09218..5f7b58995 100644
--- a/tests/libsample/modifications.h
+++ b/tests/libsample/modifications.h
@@ -93,6 +93,8 @@ public:
 
     // change the name of this virtual method
     virtual const char* className();
+
+    Point sumPointArray(int arraySize, const Point pointArray[]);
 };
 
 class LIBSAMPLE_API AbstractModifications : public Modifications
diff --git a/tests/samplebinding/modifications_test.py b/tests/samplebinding/modifications_test.py
index 9a33c25e1..b23165ff6 100644
--- a/tests/samplebinding/modifications_test.py
+++ b/tests/samplebinding/modifications_test.py
@@ -24,9 +24,8 @@
 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
 # 02110-1301 USA
 
-'''Test cases for method modifications performed as described on typesystem. '''
+'''Test cases for method modifications performed as described on type system. '''
 
-import sys
 import unittest
 
 from sample import Modifications, Point
@@ -40,7 +39,7 @@ class ExtModifications(Modifications):
 
 
 class ModificationsTest(unittest.TestCase):
-    '''Test cases for method modifications performed as described on typesystem. '''
+    '''Test cases for method modifications performed as described on type system. '''
 
     def setUp(self):
         self.mods = Modifications()
@@ -105,7 +104,7 @@ class ModificationsTest(unittest.TestCase):
         self.assertEqual(self.mods.timesTen(), 100)
 
     def testArgumentRemovalAndReturnTypeModificationWithTypesystemTemplates1(self):
-        '''Test modifications to method signature and return value using typesystem templates (case 1).'''
+        '''Test modifications to method signature and return value using type system templates (case 1).'''
         result, ok = self.mods.pointToPair(Point(2, 5))
         self.assertEqual(type(ok), bool)
         self.assertEqual(type(result), tuple)
@@ -116,7 +115,7 @@ class ModificationsTest(unittest.TestCase):
         self.assertEqual(result[1], 5.0)
 
     def testArgumentRemovalAndReturnTypeModificationWithTypesystemTemplates2(self):
-        '''Test modifications to method signature and return value using typesystem templates (case 2).'''
+        '''Test modifications to method signature and return value using type system templates (case 2).'''
         result, ok = self.mods.multiplyPointCoordsPlusValue(Point(2, 5), 4.1)
         self.assertEqual(type(ok), bool)
         self.assertEqual(type(result), float)
@@ -134,6 +133,10 @@ class ModificationsTest(unittest.TestCase):
         self.assertRaises(TypeError, self.mods.overloaded, 1, True, Point(2, 3), Point(4, 5))
         self.assertEqual(self.mods.over(1, True, Point(2, 3), Point(4, 5)), Modifications.Overloaded_ibPP)
 
+    def testPointArrayModification(self):
+        points = (Point(1, 1), Point(2, 2))
+        summedPoint = Point(1, 1) + Point(2, 2)
+        self.assertEqual(self.mods.sumPointArray(points), summedPoint)
+
 if __name__ == '__main__':
     unittest.main()
-
diff --git a/tests/samplebinding/typesystem_sample.xml b/tests/samplebinding/typesystem_sample.xml
index 5184526e4..9624ba27a 100644
--- a/tests/samplebinding/typesystem_sample.xml
+++ b/tests/samplebinding/typesystem_sample.xml
@@ -664,6 +664,23 @@
 
         <!-- change the name of this virtual method -->
         <modify-function signature="className()" rename="name"/>
+
+        <modify-function signature="sumPointArray(int, const Point*)">
+            <modify-argument index="1">
+                <remove-argument/>
+                <conversion-rule class="native">
+                int %out = PySequence_Size(%PYARG_1);
+                </conversion-rule>
+            </modify-argument>
+            <modify-argument index="2">
+                <replace-type modified-type="PySequence" />
+                <conversion-rule class="native">
+                Shiboken::AutoArrayPointer&lt;Point&gt; %out(%1);
+                for (int i = 0; i &lt; %1; ++i)
+                    %out[i] = %CONVERTTOCPP[Point](PySequence_Fast_GET_ITEM(%PYARG_1, i));
+                </conversion-rule>
+            </modify-argument>
+        </modify-function>
     </object-type>
 
     <object-type name="AbstractModifications">
@@ -936,22 +953,8 @@
                     %PYARG_0 = %CONVERTTOPYTHON[%RETURN_TYPE](retval);
                 }
             </inject-code>
-
         </modify-function>
 
-        <inject-code class="native" position="beginning">
-            // simple class to avoid mem leak inside conversion rule used in arrayMethod
-            template&lt;typename T>
-            struct ArrayDeleter {
-                ArrayDeleter(T* ptr) : m_ptr(ptr) {}
-                ~ArrayDeleter() { delete[] m_ptr; }
-                operator T*() { return m_ptr; }
-            private:
-                T* m_ptr;
-                ArrayDeleter(const ArrayDeleter&amp;);
-                ArrayDeleter&amp; operator=(ArrayDeleter&amp;);
-            };
-        </inject-code>
         <modify-function signature="arrayMethod(int, int*) const">
             <modify-argument index="1">
                 <remove-argument/>
@@ -959,24 +962,20 @@
                   int %out = PySequence_Size(%PYARG_1);
                 </conversion-rule>
             </modify-argument>
-
             <modify-argument index="2">
-              <replace-type modified-type="PySequence"/>
-
-              <conversion-rule class="native">
+                <replace-type modified-type="PySequence"/>
+                <conversion-rule class="native">
                 int numItems = PySequence_Size(%PYARG_1);
-                ArrayDeleter&lt;int> %out(new int[numItems]);
-                for (int i=0; i &lt; numItems; i++)
-                    %out[i] = %CONVERTTOCPP[int](PySequence_GetItem(%PYARG_1, i));
-              </conversion-rule>
-
-              <conversion-rule class="target">
-                PyObject *%out = PyList_New(count);
-                for (int i=0; i &lt; count; i++)
+                Shiboken::AutoArrayPointer&lt;int&gt; %out(numItems);
+                for (int i = 0; i &lt; numItems; ++i)
+                    %out[i] = %CONVERTTOCPP[int](PySequence_Fast_GET_ITEM(%PYARG_1, i));
+                </conversion-rule>
+                <conversion-rule class="target">
+                PyObject* %out = PyList_New(count);
+                for (int i = 0; i &lt; count; ++i)
                     PyList_SET_ITEM(%out, i, %CONVERTTOPYTHON[int](%in[i]));
-              </conversion-rule>
-          </modify-argument>
-
+                </conversion-rule>
+            </modify-argument>
         </modify-function>
 
         <modify-function signature="callArrayMethod(int, int*) const">
@@ -1356,10 +1355,10 @@
         <add-function signature="strBufferOverloads(Str&amp;,int)" return-type="Overload::FunctionEnum">
             <inject-code class="target" position="beginning">
                 <insert-template name="buffer_argument">
-                    <replace from="%out" to="%1_out" />
+                    <replace from="%out" to="argOut" />
                 </insert-template>
                 PyThreadState* _save = PyEval_SaveThread();
-                %RETURN_TYPE cppResult = %CPPSELF.%FUNCTION_NAME(%1_out, %2);
+                %RETURN_TYPE cppResult = %CPPSELF.%FUNCTION_NAME(argOut, %2);
                 PyEval_RestoreThread(_save);
                 %PYARG_0 = %CONVERTTOPYTHON[%RETURN_TYPE](cppResult);
             </inject-code>