Python -builtin constructors silently ignored keyword arguments.
Instead of silenty ignoring them, now a "TypeError: f() takes no keyword arguments"
exception is thrown if keyword arguments are used. Hence constructors and normal
methods/functions behave in the same way.
Closes issue #1595
diff --git a/CHANGES.current b/CHANGES.current
index 09facd9..e4f18ca 100644
--- a/CHANGES.current
+++ b/CHANGES.current
@@ -7,6 +7,13 @@
Version 4.0.2 (in progress)
===========================
+2020-01-13: wsfulton
+ [Python] #1595 Python -builtin constructors silently ignored keyword arguments.
+ Instead of silenty ignoring them, now a "TypeError: f() takes no keyword arguments"
+ exception is thrown if keyword arguments are used. Hence constructors and normal methods/
+ functions behave in the same way. Note, -keyword should be used with -builtin to obtain
+ keyword argument support.
+
2019-12-10: wsfulton
#1679 Fix parsing of C++11 identifiers with special meaning (final and override) when
they are used as part of the scope name of an identifier, such as a namespace name.
diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk
index c817bdf..00aa492 100644
--- a/Examples/test-suite/common.mk
+++ b/Examples/test-suite/common.mk
@@ -159,6 +159,7 @@
cpp_enum \
cpp_namespace \
cpp_nodefault \
+ cpp_parameters \
cpp_static \
cpp_typedef \
cpp14_binary_integer_literals \
diff --git a/Examples/test-suite/cpp_parameters.i b/Examples/test-suite/cpp_parameters.i
new file mode 100644
index 0000000..e8a4c94
--- /dev/null
+++ b/Examples/test-suite/cpp_parameters.i
@@ -0,0 +1,46 @@
+%module cpp_parameters
+
+%{
+// For Perl
+#ifdef Zero
+#undef Zero
+#endif
+%}
+%inline %{
+
+// Zero arguments
+struct Zero {
+ Zero() {}
+ int zero() { return 0; }
+ static int stat_zero() { return 0; }
+};
+// One mandatory argument
+struct One {
+ One(int a) {}
+ int one(int a) { return a; }
+ static int stat_one(int a) { return a; }
+};
+// Two mandatory arguments
+struct Two {
+ Two(int a, int b) {}
+ int two(int a, int b) { return a + b; }
+ static int stat_two(int a, int b) { return a + b; }
+};
+// Single optional argument
+struct Single {
+ Single(int a=0) {}
+ int single(int a=0) { return a; }
+ static int stat_single(int a=0) { return a; }
+};
+
+int global_zero() { return 0; }
+int global_one(int a) { return a; }
+int global_two(int a, int b) { return a + b; }
+int global_single(int a=0) { return a; }
+
+#ifdef SWIGPYTHON_BUILTIN
+bool is_python_builtin() { return true; }
+#else
+bool is_python_builtin() { return false; }
+#endif
+%}
diff --git a/Examples/test-suite/python/cpp_parameters_runme.py b/Examples/test-suite/python/cpp_parameters_runme.py
new file mode 100644
index 0000000..99d14ad
--- /dev/null
+++ b/Examples/test-suite/python/cpp_parameters_runme.py
@@ -0,0 +1,296 @@
+from cpp_parameters import *
+
+# Testing correct and incorrect parameter counts being passed (kwargs and non-kwargs)
+# Note that the implementation depends a lot on whether zero, one, two or more args are being wrapped
+
+def is_python_fastproxy():
+ """Return True if SWIG is generating Python code using -fastproxy."""
+ import cpp_parameters
+ # Note: _swig_new_instance_method is only generated when using -fastproxy
+ return hasattr(cpp_parameters, "_swig_new_instance_method")
+
+# Zero parameters expected
+x = Zero()
+try:
+ x = Zero(z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ x = Zero(0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ x.zero(z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ x.zero(0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ Zero.stat_zero(z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ Zero.stat_zero(0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ global_zero(z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ global_zero(0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+# One mandatory parameter expected
+x = One(1)
+try:
+ x = One(a=1, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ x = One(1, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ x.one(a=1, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ x.one(1, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ One.stat_one(a=1, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ One.stat_one(1, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ global_one(a=1, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ global_one(1, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+# Two mandatory parameters expected
+x = Two(1, 2)
+try:
+ x = Two(a=1, b=2, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ x = Two(1, 2, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ x.two(a=1, b=2, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ x.two(1, 2, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ Two.stat_two(a=1, b=2, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ Two.stat_two(1, 2, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ global_two(a=1, b=2, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ global_two(1, 2, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+# Single optional parameter expected
+x = Single(1)
+try:
+ x = Single(a=1, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ x = Single(1, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ x.single(a=1, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ x.single(1, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ Single.stat_single(a=1, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ Single.stat_single(1, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+try:
+ global_single(a=1, z=0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+try:
+ global_single(1, 0)
+ raise RuntimeError("Missed throw")
+except TypeError:
+ pass
+
+# Test that -builtin option throws TypeError if kwargs are used even when they look like they should work, kwargs are not supported unless using -keyword.
+# Also same for -fastproxy option except that kwargs are supported by default for constructors. TODO: Fix inconsistency.
+
+if is_python_builtin() or is_python_fastproxy():
+ # One mandatory parameter in API
+ x = One(1)
+ if is_python_fastproxy():
+ x = One(a=1)
+ else:
+ try:
+ x = One(a=1)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+ try:
+ x.one(a=1)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+ try:
+ One.stat_one(a=1)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+ try:
+ global_one(a=1)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+
+ # Two mandatory parameters in API
+ x = Two(1, 2)
+ if is_python_fastproxy():
+ x = Two(a=1, b=2)
+ else:
+ try:
+ x = Two(a=1, b=2)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+ try:
+ x.two(a=1, b=2)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+ try:
+ Two.stat_two(a=1, b=2)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+ try:
+ global_two(a=1, b=2)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+
+ # Single optional parameter in API
+ x = Single(1)
+ if is_python_fastproxy():
+ x = Single(a=1)
+ else:
+ try:
+ x = Single(a=1)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+ try:
+ x.single(a=1)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+ try:
+ Single.stat_single(a=1)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+ try:
+ global_single(a=1)
+ raise RuntimeError("Missed throw")
+ except TypeError:
+ pass
+
+else:
+ # Non-builtin should work as expected
+ # One mandatory parameter in API
+ x = One(a=1)
+ x.one(a=1)
+ One.stat_one(a=1)
+ global_one(a=1)
+
+ # Two mandatory parameters in API
+ x = Two(a=1, b=2)
+ x.two(a=1, b=2)
+ Two.stat_two(a=1, b=2)
+ global_two(a=1, b=2)
+
+ # Single optional parameter in API
+ x = Single(a=1)
+ x.single(a=1)
+ Single.stat_single(a=1)
+ global_single(a=1)
diff --git a/Lib/python/pyrun.swg b/Lib/python/pyrun.swg
index 82859b8..2180181 100644
--- a/Lib/python/pyrun.swg
+++ b/Lib/python/pyrun.swg
@@ -183,6 +183,19 @@
}
}
+SWIGINTERN int
+SWIG_Python_CheckNoKeywords(PyObject *kwargs, const char *name) {
+ int no_kwargs = 1;
+ if (kwargs) {
+ assert(PyDict_Check(kwargs));
+ if (PyDict_Size(kwargs) > 0) {
+ PyErr_Format(PyExc_TypeError, "%s() does not take keyword arguments", name);
+ no_kwargs = 0;
+ }
+ }
+ return no_kwargs;
+}
+
/* A functor is a function object with one single object argument */
#define SWIG_Python_CallFunctor(functor, obj) PyObject_CallFunctionObjArgs(functor, obj, NULL);
diff --git a/Source/Modules/python.cxx b/Source/Modules/python.cxx
index 4792090..1e211fb 100644
--- a/Source/Modules/python.cxx
+++ b/Source/Modules/python.cxx
@@ -2493,7 +2493,7 @@
String *symname = Getattr(n, "sym:name");
String *wname = Swig_name_wrapper(symname);
- const char *builtin_kwargs = builtin_ctor ? ", PyObject *SWIGUNUSEDPARM(kwargs)" : "";
+ const char *builtin_kwargs = builtin_ctor ? ", PyObject *kwargs" : "";
Printv(f->def, linkage, builtin_ctor ? "int " : "PyObject *", wname, "(PyObject *self, PyObject *args", builtin_kwargs, ") {", NIL);
Wrapper_add_local(f, "argc", "Py_ssize_t argc");
@@ -2503,6 +2503,9 @@
if (!fastunpack) {
Wrapper_add_local(f, "ii", "Py_ssize_t ii");
+ if (builtin_ctor)
+ Printf(f->code, "if (!SWIG_Python_CheckNoKeywords(kwargs, \"%s\")) SWIG_fail;\n", symname);
+
if (maxargs - (add_self ? 1 : 0) > 0) {
Append(f->code, "if (!PyTuple_Check(args)) SWIG_fail;\n");
Append(f->code, "argc = PyObject_Length(args);\n");
@@ -2518,8 +2521,9 @@
if (add_self)
Append(f->code, "argc++;\n");
} else {
- String *iname = Getattr(n, "sym:name");
- Printf(f->code, "if (!(argc = SWIG_Python_UnpackTuple(args, \"%s\", 0, %d, argv%s))) SWIG_fail;\n", iname, maxargs, add_self ? "+1" : "");
+ if (builtin_ctor)
+ Printf(f->code, "if (!SWIG_Python_CheckNoKeywords(kwargs, \"%s\")) SWIG_fail;\n", symname);
+ Printf(f->code, "if (!(argc = SWIG_Python_UnpackTuple(args, \"%s\", 0, %d, argv%s))) SWIG_fail;\n", symname, maxargs, add_self ? "+1" : "");
if (add_self)
Append(f->code, "argv[0] = self;\n");
else
@@ -2713,7 +2717,7 @@
Append(wname, overname);
}
- const char *builtin_kwargs = builtin_ctor ? ", PyObject *SWIGUNUSEDPARM(kwargs)" : "";
+ const char *builtin_kwargs = builtin_ctor ? ", PyObject *kwargs" : "";
if (!allow_kwargs || overname) {
if (!varargs) {
Printv(f->def, linkage, wrap_return, wname, "(PyObject *", self_param, ", PyObject *args", builtin_kwargs, ") {", NIL);
@@ -2886,6 +2890,7 @@
funpack = 0;
} else {
Clear(parse_args);
+
if (funpack) {
Clear(f->def);
if (overname) {
@@ -2898,6 +2903,8 @@
} else {
int is_tp_call = Equal(Getattr(n, "feature:python:slot"), "tp_call");
Printv(f->def, linkage, wrap_return, wname, "(PyObject *", self_param, ", PyObject *args", builtin_kwargs, ") {", NIL);
+ if (builtin_ctor)
+ Printf(parse_args, "if (!SWIG_Python_CheckNoKeywords(kwargs, \"%s\")) SWIG_fail;\n", iname);
if (onearg && !builtin_ctor && !is_tp_call) {
Printf(parse_args, "if (!args) SWIG_fail;\n");
Append(parse_args, "swig_obj[0] = args;\n");
@@ -2908,6 +2915,8 @@
}
}
} else {
+ if (builtin_ctor)
+ Printf(parse_args, "if (!SWIG_Python_CheckNoKeywords(kwargs, \"%s\")) SWIG_fail;\n", iname);
if (builtin && in_class && tuple_arguments == 0) {
Printf(parse_args, " if (args && PyTuple_Check(args) && PyTuple_GET_SIZE(args) > 0) SWIG_exception_fail(SWIG_TypeError, \"%s takes no arguments\");\n", iname);
} else {
