#1326 python varargs issues

[Stephen Ehmann]

Issue 1) performance concern about having an additional function layer here + doing tuple manipulation
Issue 2) the second PyTuple_GetSlice has a +1 for the last argument -- seems wrong
Issue 3) $input is not mapped to the "varargs" variable, it is mapped to "self"
Issue 4) newargs do not exist so should it be assigned a slice of the tuple, can this call be avoided and NULL passed instead of newargs?

This is the typemap and the static method I am wrapping:

%typemap(in) (...) (ArgList al)
{
    for (int j = 0; j < PyTuple_Size($input); ++j) {
        // TODO what if this fails to convert?
        boost::shared_ptr<Arg> a;
        if (ConvertPythonObjToValue(PyTuple_GetItem($input, j), a)) {
            al.push_back(a);
        }
    }

    $1 = &al;
}
class ConfigReader
{
public:
    static void writeYaml(...);
};

This is the generated code:

SWIGINTERN PyObject *_wrap_ConfigReader_writeYaml__varargs__(PyObject *SWIGUNUSEDPARM(self), PyObject *args, PyObject *varargs) {
  PyObject *resultobj = 0;
  void *arg1 = 0 ;
  ArgList al1 ;

  if(!PyArg_UnpackTuple(args,(char *)"ConfigReader_writeYaml",0,1)) SWIG_fail;
  {
    for (int j = 0; j < PyTuple_Size(self); ++j) {
      // TODO what if this fails to convert?
      boost::shared_ptr<Arg> a;
      if (ConvertPythonObjToValue(PyTuple_GetItem(self, j), a)) {
        al1.push_back(a);
      }
    }

    arg1 = &al1;
  }
  {
    try {
      ConfigReader::writeYaml(arg1);
    } catch( std::runtime_error& e ) {
      PyErr_SetString(PyExc_RuntimeError, e.what());
      return NULL;
    }
  }
  resultobj = SWIG_Py_Void();
  return resultobj;
fail:
  return NULL;
}


SWIGINTERN PyObject *_wrap_ConfigReader_writeYaml(PyObject *SWIGUNUSEDPARM(self), PyObject *args) {
  PyObject *resultobj;
  PyObject *varargs;
  PyObject *newargs;

  newargs = PyTuple_GetSlice(args,0,0);
  varargs = PyTuple_GetSlice(args,0,PyTuple_Size(args)+1);
  resultobj = _wrap_ConfigReader_writeYaml__varargs__(NULL,newargs,varargs);
  Py_XDECREF(newargs);
  Py_XDECREF(varargs);
  return resultobj;
}

[William Fulton]

C doesn't support unnamed varargs, so this is probably why you have come across issue 3. However, I see you are using writeYaml(...) in a C++ class. Although this is apparently legal, I cannot find any information how you'd use these variable arguments as va_start requires a named parameter. Can you share the implementation of writeYaml, or something similar for testing any changes?

Issue 2 is a harmless bug as Python slices will accept requests for the high value to be greater than the length of the tuple - it just truncates to the size off the tuple. I have however fixed this now.

[Stephen Ehmann]

(for some reason the double underscores got dropped from the code I pasted in above. If I need to edit the formatting of it somehow I can)

I was going off of http://www.swig.org/Doc2.0/Varargs.html#Varargs_nn6 and assumed that only having (...) was going to work and it did.

here is a typemap I use for writeYaml, ArgList is a language-agnostic intermediate representation of an args list. This is what I had posted in my original description.

%typemap(in) (...) (ArgList al)
{
    // XXX: should be PyTuple_Size($input), use varargs to WAR swig bug
    // https://sourceforge.net/p/swig/bugs/1326/
    for (int j = 0; j < PyTuple_Size(varargs); ++j) {
        // TODO what if this fails to convert?
        boost::shared_ptr<Arg> a;
        if (ConvertPythonObjToValue(PyTuple_GetItem(varargs, j), a)) {
            al.push_back(a);
        }
    }

    $1 = &al;
}

Here is the C++ header code for the method in the class:

static void writeYaml(const ArgList& args);
inline static void writeYaml(void* as) {
    writeYaml(*(reinterpret_cast<ArgList*>(as)));
}

The implementation of writeYaml just goes through the given ArgList and does some work, I don't think anything in there has anything to do with the interface. The typemap does the handling.

[William Fulton]

Your writeYaml function uses void * for the parameter, it doesn't use varargs. How do you use the vararg typemap for writeYaml?

The vararg typemaps, like in C, are a rather special case, '...' is not a type, so don't expect normal type handling. If you declare writeYaml to use varargs, then the typemap for '...' should work.

Probably you just want to write a typemap for void * or ArgList, marshalling from a Python tuple.

[Stephen Ehmann]

I don't fully understand what you mean but I'll attempt to answer. The typemap I provided does what you say in the last sentence. It converts the python tuple to my internal list which is passed via a void* to my library.

$1 from a typemap for (...) is void*, isn't that simply the case?

is there a swig varargs expert we can loop in here?

[Olly Betts]

The example supplied here seems flawed.

In C/C++, parameter(s) passed to a ... parameter list need to be unpacked using the macros in <stdarg.h>/<cstdarg>.

The SWIG ... typemap provides a mechanism to pass parameters to a C/C++ varargs function, but that doesn't change how the C/C++ function needs to access those parameters.

Telling SWIG to just pretend that a function that really takes a void* is a varargs function isn't a good idea. If the underlying API being wrapped takes const ArgList& args then just write a custom typemap for that type, as William suggests.

But if you really wanted to use varargs here, you need to also set the action feature for writeYaml to map from the void* to the paramters you actually want to pass to the C/C++ varargs, like the example in Varargs.html does:

To patch everything up, you have to rewrite the underlying action code using the %feature directive like this:

%feature("action") execlp {
  char **vargs = (char **) arg3;
  result = execlp(arg1, arg2, vargs[0], vargs[1], vargs[2], vargs[3], vargs[4],
                  vargs[5], vargs[6], vargs[7], vargs[8], vargs[9], NULL);
}
int execlp(const char *path, const char *arg, ...);

[Olly Betts]

However looking at the generated code from SWIG git master, there do seem to be some problems with it:

#ifdef __cplusplus
extern "C" {
#endif
SWIGINTERN PyObject *_wrap_ConfigReader_writeYaml__varargs__(PyObject *self, PyObject *args, PyObject *varargs) {
  PyObject *resultobj = 0;
  int arg1 ;
  void *arg2 = 0 ;
  int val1 ;
  int ecode1 = 0 ;
  ArgList al2 ;
  PyObject * obj0 = 0 ;

  if (!PyArg_UnpackTuple(args, "ConfigReader_writeYaml", 1, 2, &obj0)) SWIG_fail;
  ecode1 = SWIG_AsVal_int(obj0, &val1);
  if (!SWIG_IsOK(ecode1)) {
    SWIG_exception_fail(SWIG_ArgError(ecode1), "in method '" "ConfigReader_writeYaml" "', argument " "1"" of type '" "int""'");
  } 
  arg1 = static_cast< int >(val1);
  {
    for (int j = 0; j < PyTuple_Size(self); ++j) {
      // TODO what if this fails to convert?
      boost::shared_ptr<Arg> a;
      if (ConvertPythonObjToValue(PyTuple_GetItem(self, j), a)) {
        al2.push_back(a);
      }
    }

    arg2 = &al2;
  }
  ConfigReader::writeYaml(arg1,arg2);
  resultobj = SWIG_Py_Void();
  return resultobj;
fail:
  return NULL;
}


SWIGINTERN PyObject *_wrap_ConfigReader_writeYaml(PyObject *self, PyObject *args) {
  PyObject *resultobj;
  PyObject *varargs;
  PyObject *newargs;

  newargs = PyTuple_GetSlice(args, 0, 1);
  varargs = PyTuple_GetSlice(args, 1, PyTuple_Size(args));
  resultobj = _wrap_ConfigReader_writeYaml__varargs__(NULL, newargs, varargs);
  Py_XDECREF(newargs);
  Py_XDECREF(varargs);
  return resultobj;
}

_wrap_ConfigReader_writeYaml__varargs__:

• tries to unpack from self but this is a static method.
• doesn't touch varargs.
• passes int, void*to the C++ static method - where does that int come from?

python_varargs_typemap in the testsuite passes (and has a _runme.py) so I think these must be issues with ... without a parameter before it (but haven't investigated in any detail yet).

Such a method is indeed valid C++, but as William notes it's unclear how you'd actually unpack parameters in such a function. The answer to that is you don't! It seems this is allowed because it's useful for SFINAE tricks - search for detect( in https://www.linkedin.com/pulse/modern-c-variadic-functions-how-shoot-yourself-foot-avoid-zinin for details.

SWIG should really handle this case better.

[Olly Betts]

where does that int come from?

Oh, that's my fault - I was experimenting with the testcase and had added an int parameter before the ...!

The other issues still seem to be the case with current git master.