From: Stefan Z. <ste...@am...> - 2011-02-19 18:48:57
|
> When -fastdispatch is used, it can produce unreachable code in dispatch functions where one of the overloaded function signatures has varargs. For example, this interface: > > %inline %{ > extern int func (int i, double j); > extern int func (const char *fmt, ...); > %} > > ... produces this dispatch code: > > if (argc >= 1) { > return _wrap_func__SWIG_1(self, args); > } > if (argc == 2) { > return _wrap_func__SWIG_0(self, args); > } > > With this patch, fast dispatch is automatically disabled for any overloaded function that has varargs in one of its signatures. > > > ---------------------------------------------------------------------- > > >Comment By: William Fulton (wsfulton) > Date: 2011-02-18 21:26 > > Message: > Thanks. Instead I've fixed the problem so that varargs can still be used in > overloaded functions with -fastdispatch. Tested with the python test-suite > with -fastdispatch set. > > ---------------------------------------------------------------------- Hmm... looks like your fix will only dispatch to the varargs method if it's called with only the minimum number of arguments, i.e., no variable-length arguments are used. In some sense, that's probably a good idea, because swig doesn't do anything useful with the variable-length arguments. But it is a change in swig's behavior, and it probably ought to be documented somewhere. Stefan |
From: William S F. <ws...@fu...> - 2011-02-19 19:21:55
|
On 19/02/11 18:48, Stefan Zager wrote: > >> When -fastdispatch is used, it can produce unreachable code in dispatch functions where one of the overloaded function signatures has varargs. For example, this interface: >> >> %inline %{ >> extern int func (int i, double j); >> extern int func (const char *fmt, ...); >> %} >> >> ... produces this dispatch code: >> >> if (argc>= 1) { >> return _wrap_func__SWIG_1(self, args); >> } >> if (argc == 2) { >> return _wrap_func__SWIG_0(self, args); >> } >> >> With this patch, fast dispatch is automatically disabled for any overloaded function that has varargs in one of its signatures. >> >> >> ---------------------------------------------------------------------- >> >>> Comment By: William Fulton (wsfulton) >> Date: 2011-02-18 21:26 >> >> Message: >> Thanks. Instead I've fixed the problem so that varargs can still be used in >> overloaded functions with -fastdispatch. Tested with the python test-suite >> with -fastdispatch set. >> >> ---------------------------------------------------------------------- > > Hmm... looks like your fix will only dispatch to the varargs method if it's > called with only the minimum number of arguments, i.e., no variable-length > arguments are used. > > In some sense, that's probably a good idea, because swig doesn't do anything > useful with the variable-length arguments. But it is a change in swig's > behavior, and it probably ought to be documented somewhere. > Do you have a test case that demonstrates the change in behaviour? I couldn't find any other than ones that were previously broken. If you read the chapter on varargs in the docs, you'll see that there is no such thing as varargs by default, as the '...' is effectively ignored. If %varargs is used to extend the number of arguments, then that should work too. I should have put in a test case for that, so just have. William |
From: Stefan Z. <ste...@am...> - 2011-02-20 20:20:23
|
On Sat, 19 Feb 2011, William S Fulton wrote: > >> ---------------------------------------------------------------------- > >> > >>> Comment By: William Fulton (wsfulton) > >> Date: 2011-02-18 21:26 > >> > >> Message: > >> Thanks. Instead I've fixed the problem so that varargs can still be used in > >> overloaded functions with -fastdispatch. Tested with the python test-suite > >> with -fastdispatch set. > >> > >> ---------------------------------------------------------------------- > > > > Hmm... looks like your fix will only dispatch to the varargs method if it's > > called with only the minimum number of arguments, i.e., no variable-length > > arguments are used. > > > > In some sense, that's probably a good idea, because swig doesn't do anything > > useful with the variable-length arguments. But it is a change in swig's > > behavior, and it probably ought to be documented somewhere. > > > Do you have a test case that demonstrates the change in behaviour? I > couldn't find any other than ones that were previously broken. If you > read the chapter on varargs in the docs, you'll see that there is no > such thing as varargs by default, as the '...' is effectively ignored. > If %varargs is used to extend the number of arguments, then that should > work too. I should have put in a test case for that, so just have. Try this interface: %module varargs_overload %inline %{ int func (int arg1) { puts("func(int)\n"); } int func (const char *fmt, ...) { puts("func(const char *, ...)\n"); } %} ... and this python runme script: import varargs_overload varargs_overload.func(1) varargs_overload.func("Format string", 1, "two", 3.0) Before your fix, the runme script executes successfully, although the extra arguments aren't passed to the C-language func. With your fix, the script errors out with an exception. Stefan |
From: William S F. <ws...@fu...> - 2011-02-21 07:10:27
|
On 20/02/11 20:20, Stefan Zager wrote: > On Sat, 19 Feb 2011, William S Fulton wrote: > >>>> ---------------------------------------------------------------------- >>>> >>>>> Comment By: William Fulton (wsfulton) >>>> Date: 2011-02-18 21:26 >>>> >>>> Message: >>>> Thanks. Instead I've fixed the problem so that varargs can still be used in >>>> overloaded functions with -fastdispatch. Tested with the python test-suite >>>> with -fastdispatch set. >>>> >>>> ---------------------------------------------------------------------- >>> >>> Hmm... looks like your fix will only dispatch to the varargs method if it's >>> called with only the minimum number of arguments, i.e., no variable-length >>> arguments are used. >>> >>> In some sense, that's probably a good idea, because swig doesn't do anything >>> useful with the variable-length arguments. But it is a change in swig's >>> behavior, and it probably ought to be documented somewhere. >>> >> Do you have a test case that demonstrates the change in behaviour? I >> couldn't find any other than ones that were previously broken. If you >> read the chapter on varargs in the docs, you'll see that there is no >> such thing as varargs by default, as the '...' is effectively ignored. >> If %varargs is used to extend the number of arguments, then that should >> work too. I should have put in a test case for that, so just have. > > > Try this interface: > > %module varargs_overload > > %inline %{ > int func (int arg1) > { puts("func(int)\n"); } > > int func (const char *fmt, ...) > { puts("func(const char *, ...)\n"); } > %} > > > ... and this python runme script: > > import varargs_overload > > varargs_overload.func(1) > varargs_overload.func("Format string", 1, "two", 3.0) > > > > Before your fix, the runme script executes successfully, although the extra > arguments aren't passed to the C-language func. With your fix, the script > errors out with an exception. > Indeed. I'd say this was okay as it now correctly errors out instead of silently ignoring arguments. Also the erroring out behaviour now matches that of a vararg function that is not overloaded. You are right about documenting the change in behaviour and an extra note in the CHANGES file about this aspect of the bug fix would have been useful, but unfortunately it is too late. But at least the behaviour is now consistent with the official varargs documentation, even if it doesn't explicitly mention overloaded vararg functions. William |