improve error reporting on mapped keyword arguments for C functions, implement out...

diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 6ee1d05..ad75581 100755 (executable)
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -4343,15 +4343,18 @@ class GeneralCallNode(CallNode):
                 self.type = error_type
                 return self
             if hasattr(self.function, 'entry'):
-                node = self.map_keywords_to_posargs(env)
-                if node is not None:
+                node = self.map_to_simple_call_node()
+                if node is not None and node is not self:
                     return node.analyse_types(env)
+                elif self.function.entry.as_variable:
+                    self.function = self.function.coerce_to_pyobject(env)
+                elif node is self:
+                    error(self.pos,
+                          "Non-trivial keyword arguments and starred "
+                          "arguments not allowed in cdef functions.")
                 else:
-                    if self.function.entry.as_variable:
-                        self.function = self.function.coerce_to_pyobject(env)
-                    else:
-                        error(self.pos, "Keyword and starred arguments "
-                                        "not allowed in cdef functions.")
+                    # error was already reported
+                    pass
             else:
                 self.function = self.function.coerce_to_pyobject(env)
         if self.keyword_args:
@@ -4371,74 +4374,133 @@ class GeneralCallNode(CallNode):
         self.is_temp = 1
         return self
 
-    def map_keywords_to_posargs(self, env):
+    def map_to_simple_call_node(self):
+        """
+        Tries to map keyword arguments to declared positional arguments.
+        Returns self to try a Python call, None to report an error
+        or a SimpleCallNode if the mapping succeeds.
+        """
         if not isinstance(self.positional_args, TupleNode):
             # has starred argument
-            return None
+            return self
         if not isinstance(self.keyword_args, DictNode):
-            # nothing to do here
-            return None
+            # keywords come from arbitrary expression => nothing to do here
+            return self
         function = self.function
         entry = getattr(function, 'entry', None)
         if not entry or not entry.is_cfunction:
-            return None
+            return self
 
-        args = self.positional_args.args
+        pos_args = self.positional_args.args
         kwargs = self.keyword_args
         declared_args = entry.type.args
-        if len(declared_args) < len(args):
-            # will lead to an error elsewhere
+        if entry.is_cmethod:
+            declared_args = declared_args[1:] # skip 'self'
+
+        if len(pos_args) > len(declared_args):
             error(self.pos, "function call got too many positional arguments, "
-                            "expected %d, got %s" % (len(declared_args), len(args)))
+                            "expected %d, got %s" % (len(declared_args),
+                                                     len(pos_args)))
             return None
 
-        matched_pos_args = set([arg.name for arg in declared_args[:len(args)]])
-        unmatched_args = declared_args[len(args):]
-        matched_kwargs = set()
-        args = list(args)
+        matched_args = set([ arg.name for arg in declared_args[:len(pos_args)]
+                             if arg.name ])
+        unmatched_args = declared_args[len(pos_args):]
+        matched_kwargs_count = 0
+        args = list(pos_args)
+
+        # check for duplicate keywords
+        seen = set(matched_args)
+        has_errors = False
+        for arg in kwargs.key_value_pairs:
+            name = arg.key.value
+            if name in seen:
+                error(arg.pos, "argument '%s' passed twice" % name)
+                has_errors = True
+                # continue to report more errors if there are any
+            seen.add(name)
 
         # match keywords that are passed in order
         for decl_arg, arg in zip(unmatched_args, kwargs.key_value_pairs):
             name = arg.key.value
-            if name in matched_pos_args:
-                error(arg.pos, "keyword argument '%s' passed twice" % name)
-                return None
             if decl_arg.name == name:
-                matched_kwargs.add(name)
+                matched_args.add(name)
+                matched_kwargs_count += 1
                 args.append(arg.value)
             else:
                 break
 
-        # match simple keyword arguments that are passed out of order
-        if len(kwargs.key_value_pairs) > len(matched_kwargs):
+        # match keyword arguments that are passed out-of-order, but keep
+        # the evaluation of non-simple arguments in order by moving them
+        # into temps
+        from Cython.Compiler.UtilNodes import EvalWithTempExprNode, LetRefNode
+        temps = []
+        if len(kwargs.key_value_pairs) > matched_kwargs_count:
             unmatched_args = declared_args[len(args):]
-            keywords = dict([ (arg.key.value, arg.value)
-                              for arg in kwargs.key_value_pairs ])
+            keywords = dict([ (arg.key.value, (i, arg))
+                              for i, arg in enumerate(kwargs.key_value_pairs,
+                                                      len(pos_args)) ])
+            first_missing_keyword = None
             for decl_arg in unmatched_args:
                 name = decl_arg.name
-                arg_value = keywords.get(name)
-                if arg_value and arg_value.is_simple():
-                    matched_kwargs.add(name)
-                    args.append(arg_value)
+                if name not in keywords:
+                    # missing keyword argument => either done or error
+                    if not first_missing_keyword:
+                        first_missing_keyword = name
+                    continue
+                elif first_missing_keyword:
+                    # wasn't the last keyword => gaps are not supported
+                    error(self.pos, "C function call is missing "
+                                    "argument '%s'" % first_missing_keyword)
+                    return None
+                pos, arg = keywords[name]
+                matched_args.add(name)
+                matched_kwargs_count += 1
+                if arg.value.is_simple():
+                    args.append(arg.value)
                 else:
-                    # first missing keyword argument
-                    break
-
-        # TODO: match keywords out-of-order and move values
-        #       into ordered temps if necessary
+                    temp = LetRefNode(arg.value)
+                    assert temp.is_simple()
+                    args.append(temp)
+                    temps.append((pos, temp))
+
+            if temps:
+                # may have to move preceding non-simple args into temps
+                final_args = []
+                new_temps = []
+                first_temp_arg = temps[0][-1]
+                for arg_value in args:
+                    if arg_value is first_temp_arg:
+                        break  # done
+                    if arg_value.is_simple():
+                        final_args.append(arg_value)
+                    else:
+                        temp = LetRefNode(arg_value)
+                        new_temps.append(temp)
+                        final_args.append(temp)
+                if new_temps:
+                    args = final_args
+                temps = new_temps + [ arg for i,arg in sorted(temps) ]
+
+        # check for unexpected keywords
+        for arg in kwargs.key_value_pairs:
+            name = arg.key.value
+            if name not in matched_args:
+                has_errors = True
+                error(arg.pos,
+                      "C function got unexpected keyword argument '%s'" %
+                      name)
 
-        if not matched_kwargs:
+        if has_errors:
+            # error was reported already
             return None
-        self.positional_args.args = args
-        if len(kwargs.key_value_pairs) == len(matched_kwargs):
-            # all keywords mapped => only positional arguments left
-            return SimpleCallNode(
-                self.pos, function=function, args=args)
-
-        kwargs.key_value_pairs = [
-            item for item in kwargs.key_value_pairs
-            if item.key.value not in matched_kwargs ]
-        return None
+
+        # all keywords mapped to positional arguments
+        # if we are missing arguments, SimpleCallNode will figure it out
+        node = SimpleCallNode(self.pos, function=function, args=args)
+        for temp in temps[::-1]:
+            node = EvalWithTempExprNode(temp, node)
+        return node
 
     def generate_result_code(self, code):
         if self.type.is_error: return

diff --git a/tests/errors/e_cdef_keywords_T241.pyx b/tests/errors/e_cdef_keywords_T241.pyx
index 7c45b89..37fca60 100644 (file)
--- a/tests/errors/e_cdef_keywords_T241.pyx
+++ b/tests/errors/e_cdef_keywords_T241.pyx
@@ -8,14 +8,62 @@ cdef class A:
     cdef some_method(self, x, y=1):
         pass
 
+# ok
 some_function(1, 2)
 some_function(1, y=2)
 
+# nok
+some_function(1, x=1)
+some_function(1, x=2, y=2)
+some_function(1, y=2, z=3)
+some_function(1, z=3)
+some_function(1, 2, z=3)
+some_function(x=1, y=2, z=3)
+some_function(x=1, y=2, x=1)
+some_function(x=1, y=2, x=1, z=3)
+
 cdef A a = A()
+# ok
 a.some_method(1)
 a.some_method(1, 2)
 a.some_method(1, y=2)
+a.some_method(x=1, y=2)
+
+# nok
+a.some_method(1, x=1)
+a.some_method(1, 2, x=1)
+a.some_method(1, 2, y=2)
+a.some_method(1, 2, x=1, y=2)
+a.some_method(1, 2, y=2, x=1)
+a.some_method(1, y=2, x=1)
+a.some_method(1, 2, z=3)
+a.some_method(1, y=2, z=3)
+a.some_method(x=1, x=1)
+a.some_method(x=1, x=1, y=2)
+a.some_method(x=1, y=2, x=1)
+
 
 _ERRORS = u"""
-17:13: Keyword and starred arguments not allowed in cdef functions.
+16:18: argument 'x' passed twice
+17:18: argument 'x' passed twice
+18:23: C function got unexpected keyword argument 'z'
+19:18: C function got unexpected keyword argument 'z'
+20:21: C function got unexpected keyword argument 'z'
+21:25: C function got unexpected keyword argument 'z'
+22:25: argument 'x' passed twice
+23:25: argument 'x' passed twice
+23:30: C function got unexpected keyword argument 'z'
+33:18: argument 'x' passed twice
+34:21: argument 'x' passed twice
+35:21: argument 'y' passed twice
+36:21: argument 'x' passed twice
+36:26: argument 'y' passed twice
+37:21: argument 'y' passed twice
+37:26: argument 'x' passed twice
+38:23: argument 'x' passed twice
+39:21: C function got unexpected keyword argument 'z'
+40:23: C function got unexpected keyword argument 'z'
+41:20: argument 'x' passed twice
+42:20: argument 'x' passed twice
+43:25: argument 'x' passed twice
 """

diff --git a/tests/run/simplify_calls.pyx b/tests/run/simplify_calls.pyx
index 32224b4..409f65a 100644 (file)
--- a/tests/run/simplify_calls.pyx
+++ b/tests/run/simplify_calls.pyx
@@ -43,7 +43,6 @@ def cfunc_some_keywords_unordered():
     return cfunc(1, 2, d=4, c=3)
 
 
-'''
 @cython.test_fail_if_path_exists('//GeneralCallNode')
 @cython.test_assert_path_exists('//SimpleCallNode')
 def cfunc_some_keywords_unordered_sideeffect():
@@ -55,7 +54,6 @@ def cfunc_some_keywords_unordered_sideeffect():
     [4, 3]
     """
     return cfunc(1, 2, d=side_effect(4), c=side_effect(3))
-'''
 
 
 @cython.test_fail_if_path_exists('//GeneralCallNode')