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

CHANGES.current

@@ -7,6 +7,13 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/
 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.

Examples/test-suite/common.mk

@@ -159,6 +159,7 @@ CPP_TEST_CASES += \
 	cpp_enum \
 	cpp_namespace \
 	cpp_nodefault \
+	cpp_parameters \
 	cpp_static \
 	cpp_typedef \
 	cpp14_binary_integer_literals \

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
+%}

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)

Lib/python/pyrun.swg

@@ -183,6 +183,19 @@ SWIG_Python_UnpackTuple(PyObject *args, const char *name, Py_ssize_t min, Py_ssi
   }
 }
 
+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);
 

Source/Modules/python.cxx

@@ -2493,7 +2493,7 @@ public:
     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 @@ public:
     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 @@ public:
       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 @@ public:
       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 @@ public:
       funpack = 0;
     } else {
       Clear(parse_args);
+
       if (funpack) {
 	Clear(f->def);
 	if (overname) {
@@ -2898,6 +2903,8 @@ public:
 	} 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 @@ public:
 	  }
 	}
       } 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 {