optimise KeywordArgsNode away when there are no additional keyword arguments to merge

optimise kwargs dict creation away when it's only being passed on and not otherwise modified (ticket 744)

Cython/Compiler/ExprNodes.py

@@ -7442,6 +7442,7 @@ class Py3ClassNode(ExprNode):
                 code.error_goto_if_null(self.result(), self.pos)))
         code.put_gotref(self.py_result())
 
+
 class KeywordArgsNode(ExprNode):
     #  Helper class for keyword arguments.
     #
@@ -7484,11 +7485,17 @@ class KeywordArgsNode(ExprNode):
     def analyse_types(self, env):
         arg = self.starstar_arg.analyse_types(env)
         arg = arg.coerce_to_pyobject(env)
-        self.starstar_arg = arg.as_none_safe_node(
+        arg = arg.as_none_safe_node(
             # FIXME: CPython's error message starts with the runtime function name
             'argument after ** must be a mapping, not NoneType')
-        self.keyword_args = [ item.analyse_types(env)
-                              for item in self.keyword_args ]
+        self.starstar_arg = arg
+        if not self.keyword_args and arg.type is dict_type:
+            # strip this intermediate node and use the bare dict
+            if arg.is_name and arg.entry.is_arg and len(arg.entry.cf_assignments) == 1:
+                # passing **kwargs through to function call => allow NULL
+                arg.allow_null = True
+            return arg
+        self.keyword_args = [item.analyse_types(env) for item in self.keyword_args]
         return self
 
     def may_be_none(self):
@@ -7500,7 +7507,7 @@ class KeywordArgsNode(ExprNode):
         code.mark_pos(self.pos)
         self.allocate_temp_result(code)
         self.starstar_arg.generate_evaluation_code(code)
-        if self.starstar_arg.type is not Builtin.dict_type:
+        if self.starstar_arg.type is not dict_type:
             # CPython supports calling functions with non-dicts, so do we
             code.putln('if (likely(PyDict_Check(%s))) {' %
                        self.starstar_arg.py_result())
@@ -7516,7 +7523,7 @@ class KeywordArgsNode(ExprNode):
                 self.result(),
                 self.starstar_arg.py_result()))
             code.put_incref(self.result(), py_object_type)
-        if self.starstar_arg.type is not Builtin.dict_type:
+        if self.starstar_arg.type is not dict_type:
             code.putln('} else {')
             code.putln(
                 "%s = PyObject_CallFunctionObjArgs("
@@ -7557,6 +7564,7 @@ class KeywordArgsNode(ExprNode):
         for item in self.keyword_args:
             item.annotate(code)
 
+
 class PyClassMetaclassNode(ExprNode):
     # Helper class holds Python3 metaclass object
     #

Cython/Compiler/Nodes.py

@@ -3395,14 +3395,29 @@ class DefNodeWrapper(FuncDefNode):
                 bool(self.starstar_arg), self.error_value()))
 
         if self.starstar_arg:
-            code.putln("%s = (%s) ? PyDict_Copy(%s) : PyDict_New();" % (
-                    self.starstar_arg.entry.cname,
-                    Naming.kwds_cname,
-                    Naming.kwds_cname))
-            code.putln("if (unlikely(!%s)) return %s;" % (
-                    self.starstar_arg.entry.cname, self.error_value()))
-            self.starstar_arg.entry.xdecref_cleanup = 0
-            code.put_gotref(self.starstar_arg.entry.cname)
+            allow_null = all(ref.node.allow_null for ref in self.starstar_arg.entry.cf_references)
+            if allow_null:
+                code.putln("if (%s) {" % kwarg_check)
+                code.putln("%s = PyDict_Copy(%s); if (unlikely(!%s)) return %s;" % (
+                        self.starstar_arg.entry.cname,
+                        Naming.kwds_cname,
+                        self.starstar_arg.entry.cname,
+                        self.error_value()))
+                code.put_gotref(self.starstar_arg.entry.cname)
+                code.putln("} else {")
+                code.putln("%s = NULL;" % (
+                    self.starstar_arg.entry.cname,))
+                code.putln("}")
+                self.starstar_arg.entry.xdecref_cleanup = 1
+            else:
+                code.putln("%s = (%s) ? PyDict_Copy(%s) : PyDict_New();" % (
+                        self.starstar_arg.entry.cname,
+                        Naming.kwds_cname,
+                        Naming.kwds_cname))
+                code.putln("if (unlikely(!%s)) return %s;" % (
+                        self.starstar_arg.entry.cname, self.error_value()))
+                self.starstar_arg.entry.xdecref_cleanup = 0
+                code.put_gotref(self.starstar_arg.entry.cname)
 
         if self.self_in_stararg and not self.target.is_staticmethod:
             # need to create a new tuple with 'self' inserted as first item
@@ -4189,9 +4204,9 @@ class PyClassDefNode(ClassDefNode):
                         self.metaclass = item.value
                         del keyword_args.key_value_pairs[i]
             if starstar_arg:
-                self.mkw = ExprNodes.KeywordArgsNode(
+                self.mkw = ExprNodes.ProxyNode(ExprNodes.KeywordArgsNode(
                     pos, keyword_args=keyword_args and keyword_args.key_value_pairs or [],
-                    starstar_arg=starstar_arg)
+                    starstar_arg=starstar_arg))
             elif keyword_args.key_value_pairs:
                 self.mkw = keyword_args
             else:

Cython/Utility/ObjectHandling.c

@@ -762,7 +762,7 @@ static PyObject *__Pyx_Py3MetaclassGet(PyObject *bases, PyObject *mkw); /*proto*
 //@requires: CalculateMetaclass
 
 static PyObject *__Pyx_Py3MetaclassGet(PyObject *bases, PyObject *mkw) {
-    PyObject *metaclass = PyDict_GetItem(mkw, PYIDENT("metaclass"));
+    PyObject *metaclass = mkw ? PyDict_GetItem(mkw, PYIDENT("metaclass")) : NULL;
     if (metaclass) {
         Py_INCREF(metaclass);
         if (PyDict_DelItem(mkw, PYIDENT("metaclass")) < 0) {

tests/run/kwargs_passthrough.pyx

+cimport cython
+
+
+@cython.test_fail_if_path_exists('//KeywordArgsNode')
+def wrap_passthrough(f):
+    """
+    >>> def f(a=1): return a
+    >>> wrapped = wrap_passthrough(f)
+    >>> wrapped(1)
+    CALLED
+    1
+    >>> wrapped(a=2)
+    CALLED
+    2
+    """
+    def wrapper(*args, **kwargs):
+        print("CALLED")
+        return f(*args, **kwargs)
+    return wrapper
+
+
+@cython.test_assert_path_exists('//KeywordArgsNode')
+def wrap_passthrough_more(f):
+    """
+    >>> def f(a=1, test=2):
+    ...     return a, test
+    >>> wrapped = wrap_passthrough_more(f)
+    >>> wrapped(1)
+    CALLED
+    (1, 1)
+    >>> wrapped(a=2)
+    CALLED
+    (2, 1)
+    """
+    def wrapper(*args, **kwargs):
+        print("CALLED")
+        return f(*args, test=1, **kwargs)
+    return wrapper
+
+
+@cython.test_fail_if_path_exists('//KeywordArgsNode')
+def wrap_passthrough2(f):
+    """
+    >>> def f(a=1): return a
+    >>> wrapped = wrap_passthrough2(f)
+    >>> wrapped(1)
+    CALLED
+    1
+    >>> wrapped(a=2)
+    CALLED
+    2
+    """
+    def wrapper(*args, **kwargs):
+        print("CALLED")
+        f(*args, **kwargs)
+        return f(*args, **kwargs)
+    return wrapper
+
+
+@cython.test_fail_if_path_exists('//KeywordArgsNode')
+def wrap_modify(f):
+    """
+    >>> def f(a=1, test=2):
+    ...     return a, test
+
+    >>> wrapped = wrap_modify(f)
+    >>> wrapped(1)
+    CALLED
+    (1, 1)
+    >>> wrapped(a=2)
+    CALLED
+    (2, 1)
+    >>> wrapped(a=2, test=3)
+    CALLED
+    (2, 1)
+    """
+    def wrapper(*args, **kwargs):
+        print("CALLED")
+        kwargs['test'] = 1
+        return f(*args, **kwargs)
+    return wrapper
+
+
+@cython.test_fail_if_path_exists('//KeywordArgsNode')
+def wrap_modify_mix(f):
+    """
+    >>> def f(a=1, test=2):
+    ...     return a, test
+
+    >>> wrapped = wrap_modify_mix(f)
+    >>> wrapped(1)
+    CALLED
+    (1, 1)
+    >>> wrapped(a=2)
+    CALLED
+    (2, 1)
+    >>> wrapped(a=2, test=3)
+    CALLED
+    (2, 1)
+    """
+    def wrapper(*args, **kwargs):
+        print("CALLED")
+        f(*args, **kwargs)
+        kwargs['test'] = 1
+        return f(*args, **kwargs)
+    return wrapper
+
+
+@cython.test_assert_path_exists('//KeywordArgsNode')
+def wrap_modify_func(f):
+    """
+    >>> def f(a=1, test=2):
+    ...     return a, test
+
+    >>> wrapped = wrap_modify_func(f)
+    >>> wrapped(1)
+    CALLED
+    (1, 1)
+    >>> wrapped(a=2)
+    CALLED
+    (2, 1)
+    >>> wrapped(a=2, test=3)
+    CALLED
+    (2, 1)
+    """
+    def modify(kw):
+        kw['test'] = 1
+        return kw
+
+    def wrapper(*args, **kwargs):
+        print("CALLED")
+        return f(*args, **modify(kwargs))
+    return wrapper
+
+
+@cython.test_assert_path_exists('//KeywordArgsNode')
+def wrap_modify_func_mix(f):
+    """
+    >>> def f(a=1, test=2):
+    ...     return a, test
+
+    >>> wrapped = wrap_modify_func_mix(f)
+    >>> wrapped(1)
+    CALLED
+    (1, 1)
+    >>> wrapped(a=2)
+    CALLED
+    (2, 1)
+    >>> wrapped(a=2, test=3)
+    CALLED
+    (2, 1)
+    """
+    def modify(kw):
+        kw['test'] = 1
+        return kw
+
+    def wrapper(*args, **kwargs):
+        print("CALLED")
+        f(*args, **kwargs)
+        return f(*args, **modify(kwargs))
+    return wrapper
+
+
+@cython.test_fail_if_path_exists('//KeywordArgsNode')
+def wrap_reassign(f):
+    """
+    >>> def f(a=1, test=2):
+    ...     return a, test
+
+    >>> wrapped = wrap_reassign(f)
+    >>> wrapped(1)
+    CALLED
+    (1, 1)
+    >>> wrapped(a=2)
+    CALLED
+    (1, 1)
+    >>> wrapped(a=2, test=3)
+    CALLED
+    (1, 1)
+    """
+    def wrapper(*args, **kwargs):
+        print("CALLED")
+        kwargs = {'test': 1}
+        return f(*args, **kwargs)
+    return wrapper
+
+
+@cython.test_fail_if_path_exists('//KeywordArgsNode')
+def kwargs_metaclass(**kwargs):
+    """
+    >>> K = kwargs_metaclass()
+    >>> K = kwargs_metaclass(metaclass=type)
+    """
+    class K(**kwargs):
+        pass
+    return K