Merge github.com:cython/cython

Cython/Compiler/ExprNodes.py

@@ -2920,6 +2920,13 @@ class SimpleCallNode(CallNode):
             func_type = func_type.base_type
         return func_type
 
+    def is_simple(self):
+        # C function calls could be considered simple, but they may
+        # have side-effects that may hit when multiple operations must
+        # be effected in order, e.g. when constructing the argument
+        # sequence for a function call or comparing values.
+        return False
+
     def analyse_c_function_call(self, env):
         if self.function.type is error_type:
             self.type = error_type
@@ -2958,16 +2965,39 @@ class SimpleCallNode(CallNode):
             self.has_optional_args = 1
             self.is_temp = 1
         # Coerce arguments
+        some_args_in_temps = False
         for i in range(min(max_nargs, actual_nargs)):
             formal_type = func_type.args[i].type
-            arg = self.args[i].coerce_to(formal_type, env)
-            if arg.type.is_pyobject and not env.nogil and (arg.is_attribute or not arg.is_simple):
-                # we do not own the argument's reference, but we must
-                # make sure it cannot be collected before we return
-                # from the function, so we create an owned temp
-                # reference to it
-                arg = arg.coerce_to_temp(env)
+            arg = self.args[i].coerce_to(formal_type, env).coerce_to_simple(env)
+            if arg.is_temp:
+                some_args_in_temps = True
+            elif arg.type.is_pyobject and not env.nogil:
+                if not arg.is_name or arg.entry and (not arg.entry.is_local or arg.entry.in_closure):
+                    # we do not safely own the argument's reference,
+                    # but we must make sure it cannot be collected
+                    # before we return from the function, so we create
+                    # an owned temp reference to it
+                    some_args_in_temps = True
+                    arg = arg.coerce_to_temp(env)
             self.args[i] = arg
+        if some_args_in_temps:
+            # if some args are temps and others are not, they may get
+            # constructed in the wrong order (temps first) => make
+            # sure they are either all temps or all not temps
+            for i in range(min(max_nargs, actual_nargs)-1):
+                arg = self.args[i]
+                if arg.is_name and arg.entry and (
+                    (arg.entry.is_local and not arg.entry.in_closure)
+                    or arg.entry.type.is_cfunction):
+                    # local variables and C functions are safe
+                    pass
+                elif env.nogil and arg.type.is_pyobject:
+                    # can't copy a Python reference into a temp in nogil
+                    # env (this is safe: a construction would fail in
+                    # nogil anyway)
+                    pass
+                else:
+                    self.args[i] = arg.coerce_to_temp(env)
         for i in range(max_nargs, actual_nargs):
             arg = self.args[i]
             if arg.type.is_pyobject:
@@ -3979,6 +4009,10 @@ class TupleNode(SequenceNode):
                 self.is_temp = 0
                 self.is_literal = 1
 
+    def is_simple(self):
+        # either temp or constant => always simple
+        return True
+
     def calculate_result_code(self):
         if len(self.args) > 0:
             return self.result_code

tests/run/temp_sideeffects_T654.pyx

+
+# function call arguments
+
+arg_order = []
+
+cdef int f():
+    arg_order.append(1)
+    return 1
+
+def g():
+    arg_order.append(2)
+    return 2
+
+cdef call2(int x, object o):
+    return x, o
+
+def test_c_call():
+    """
+    >>> arg_order
+    []
+    >>> test_c_call()
+    (1, 2)
+    >>> arg_order
+    [1, 2]
+    """
+    return call2(f(), g())
+
+# module globals
+
+cdef object X = 1
+cdef redefine_global():
+    global X
+    x,X = X,2
+    return x
+
+cdef call3(object x1, int o, object x2):
+    return (x1, o, x2)
+
+def test_global_redefine():
+    """
+    >>> test_global_redefine()
+    (1, 1, 2)
+    """
+    return call3(X, redefine_global(), X)