Fix bug in Python builtin support for keyword args

The fix is when using kwargs feature or -keyword.
The fix is in the argument error checking when wrapping zero
argument constructors only. Supplied keyword args were silently
ignored.

Issue #1595

CHANGES.current

@@ -7,3 +7,7 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/
 Version 4.0.2 (in progress)
 ===========================
 
+2019-11-01: wsfulton
+            [Python] #1595 Fix bug in support for keyword arguments (kwargs feature or -keyword)
+            when using -builtin. The fix is in the argument error checking when wrapping zero
+            argument constructors only.

Examples/test-suite/kwargs_feature.i

@@ -100,3 +100,29 @@
 
   int foo_mm(int min = 1, int max = 2) {return min + max; }
 %}
+
+
+// Extended constructors
+%extend Extending0 {
+  Extending0() { return new Extending0(); }
+}
+%extend Extending1 {
+  Extending1(int one) { return new Extending1(); }
+}
+%extend Extending2 {
+  Extending2(int one, const char *two) { return new Extending2(); }
+}
+%extend ExtendingOptArgs1 {
+  ExtendingOptArgs1(int one = 0) { return new ExtendingOptArgs1(); }
+}
+%extend ExtendingOptArgs2 {
+  ExtendingOptArgs2(int one = 0, const char* two = NULL) { return new ExtendingOptArgs2(); }
+}
+
+%inline %{
+struct Extending0 {};
+struct Extending1 {};
+struct Extending2 {};
+struct ExtendingOptArgs1 {};
+struct ExtendingOptArgs2 {};
+%}

Examples/test-suite/python/kwargs_feature_runme.py

@@ -79,3 +79,46 @@ if rfoo(n=11, x=22) != -11:
 
 if rfoo(x=11, n=22) != 11:
     raise RuntimeError
+
+# Extended constructors
+e = Extending0()
+e = Extending1(one=1)
+e = Extending1(1)
+e = Extending2(1, "two")
+e = Extending2(1, two="two")
+e = Extending2(two="two", one=1)
+e = ExtendingOptArgs1()
+e = ExtendingOptArgs1(1)
+e = ExtendingOptArgs2(one=1)
+e = ExtendingOptArgs2()
+e = ExtendingOptArgs2(one=1)
+e = ExtendingOptArgs2(two="two")
+e = ExtendingOptArgs2(two="two", one=1)
+
+# Invalid kwargs test
+h = Hello()
+try:
+    h = Hello(nonexistent=10)
+    raise RuntimeError("missed exception")
+except TypeError as e:
+    pass
+
+f = Foo(1)
+f = Foo(a=1)
+try:
+    f = Foo(nonexistent=10)
+    raise RuntimeError("missed exception")
+except TypeError as e:
+    pass
+
+try:
+    f = Foo(a=1, nonexistent=10)
+    raise RuntimeError("missed exception")
+except TypeError as e:
+    pass
+
+try:
+    f = Foo(1, nonexistent=10)
+    raise RuntimeError("missed exception")
+except TypeError as e:
+    pass

Source/Modules/python.cxx

@@ -2700,8 +2700,12 @@ public:
       --tuple_required;
     }
     num_fixed_arguments = tuple_required;
+
+    // builtin handles/checks kwargs by default except in constructor wrappers so we need to explicitly handle them in the C constructor wrapper
+    // The check below is for zero arguments. Sometimes (eg directors) self is the first argument for a method with zero arguments.
     if (((num_arguments == 0) && (num_required == 0)) || ((num_arguments == 1) && (num_required == 1) && Getattr(l, "self")))
-      allow_kwargs = 0;
+      if (!builtin_ctor)
+	allow_kwargs = 0;
     varargs = emit_isvarargs(l);
 
     String *wname = Copy(wrapper_name);
@@ -2727,7 +2731,7 @@ public:
       }
       Printv(f->def, linkage, wrap_return, wname, "(PyObject *", self_param, ", PyObject *args, PyObject *kwargs) {", NIL);
     }
-    if (!builtin || !in_class || tuple_arguments > 0) {
+    if (!builtin || !in_class || tuple_arguments > 0 || builtin_ctor) {
       if (!allow_kwargs) {
 	Append(parse_args, "    if (!PyArg_ParseTuple(args, \"");
       } else {
@@ -2876,9 +2880,7 @@ public:
       Printv(f->locals, "  char * kwnames[] = ", kwargs, ";\n", NIL);
     }
 
-    if (builtin && !funpack && in_class && tuple_arguments == 0) {
+    if (use_parse || allow_kwargs) {
-      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 if (use_parse || allow_kwargs) {
       Printf(parse_args, ":%s\"", iname);
       Printv(parse_args, arglist, ")) SWIG_fail;\n", NIL);
       funpack = 0;
@@ -2906,8 +2908,12 @@ public:
 	  }
 	}
       } else {
-	Printf(parse_args, "if (!PyArg_UnpackTuple(args, \"%s\", %d, %d", iname, num_fixed_arguments, tuple_arguments);
+	if (builtin && in_class && tuple_arguments == 0) {
-	Printv(parse_args, arglist, ")) SWIG_fail;\n", NIL);
+	  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 {
+	  Printf(parse_args, "if (!PyArg_UnpackTuple(args, \"%s\", %d, %d", iname, num_fixed_arguments, tuple_arguments);
+	  Printv(parse_args, arglist, ")) SWIG_fail;\n", NIL);
+	}
       }
     }