Added mechanism for moving rather than copying C++ temps (GH-3362)

Right now it's used in a few simple cases (function call and single assignment)
Don't attempt to remove reference types.
Be wary of the "safety check" for double moves when things like result() are called twice
Made sure fallback option was genuinely c++03 complient so that test should pass on clang

4 files changed, 112 insertions, 7 deletions

diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 182d03784..e4e1fda7e 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -448,6 +448,7 @@ class ExprNode(Node):
 
     saved_subexpr_nodes = None
     is_temp = False
+    has_temp_moved = False  # if True then attempting to do anything but free the temp is invalid
     is_target = False
     is_starred = False
 
@@ -498,6 +499,22 @@ class ExprNode(Node):
         else:
             return self.calculate_result_code()
 
+    def _make_move_result_rhs(self, result, allow_move=True):
+        if (self.is_temp and allow_move and
+                self.type.is_cpp_class and not self.type.is_reference):
+            self.has_temp_moved = True
+            return "__PYX_STD_MOVE_IF_SUPPORTED({0})".format(result)
+        else:
+            return result
+
+    def move_result_rhs(self):
+        return self._make_move_result_rhs(self.result())
+
+    def move_result_rhs_as(self, type):
+        allow_move = (type and not type.is_reference and not type.needs_refcounting)
+        return self._make_move_result_rhs(self.result_as(type),
+                                          allow_move=allow_move)
+
     def pythran_result(self, type_=None):
         if is_pythran_supported_node_or_none(self):
             return to_pythran(self)
@@ -807,6 +824,9 @@ class ExprNode(Node):
             if self.result():
                 code.put_decref_clear(self.result(), self.ctype(),
                                         have_gil=not self.in_nogil_context)
+            if self.has_temp_moved:
+                code.globalstate.use_utility_code(
+                    UtilityCode.load_cached("MoveIfSupported", "CppSupport.cpp"))
         else:
             # Already done if self.is_temp
             self.generate_subexpr_disposal_code(code)
@@ -828,6 +848,10 @@ class ExprNode(Node):
             elif self.type.is_memoryviewslice:
                 code.putln("%s.memview = NULL;" % self.result())
                 code.putln("%s.data = NULL;" % self.result())
+
+            if self.has_temp_moved:
+                code.globalstate.use_utility_code(
+                    UtilityCode.load_cached("MoveIfSupported", "CppSupport.cpp"))
         else:
             self.generate_subexpr_disposal_code(code)
 
@@ -2398,7 +2422,7 @@ class NameNode(AtomicExprNode):
             if not self.type.is_memoryviewslice:
                 if not assigned:
                     if overloaded_assignment:
-                        result = rhs.result()
+                        result = rhs.move_result_rhs()
                         if exception_check == '+':
                             translate_cpp_exception(
                                 code, self.pos,
@@ -2408,7 +2432,7 @@ class NameNode(AtomicExprNode):
                         else:
                             code.putln('%s = %s;' % (self.result(), result))
                     else:
-                        result = rhs.result_as(self.ctype())
+                        result = rhs.move_result_rhs_as(self.ctype())
 
                         if is_pythran_expr(self.type):
                             code.putln('new (&%s) decltype(%s){%s};' % (self.result(), self.result(), result))
@@ -5634,6 +5658,7 @@ class SimpleCallNode(CallNode):
             self.analyse_c_function_call(env)
             if func_type.exception_check == '+':
                 self.is_temp = True
+
         return self
 
     def function_type(self):
@@ -5870,8 +5895,8 @@ class SimpleCallNode(CallNode):
         expected_nargs = max_nargs - func_type.optional_arg_count
         actual_nargs = len(self.args)
         for formal_arg, actual_arg in args[:expected_nargs]:
-                arg_code = actual_arg.result_as(formal_arg.type)
-                arg_list_code.append(arg_code)
+            arg_code = actual_arg.move_result_rhs_as(formal_arg.type)
+            arg_list_code.append(arg_code)
 
         if func_type.is_overridable:
             arg_list_code.append(str(int(self.wrapper_call or self.function.entry.is_unbound_cmethod)))
@@ -5884,7 +5909,7 @@ class SimpleCallNode(CallNode):
             arg_list_code.append(optional_args)
 
         for actual_arg in self.args[len(formal_args):]:
-            arg_list_code.append(actual_arg.result())
+            arg_list_code.append(actual_arg.move_result_rhs())
 
         result = "%s(%s)" % (self.function.result(), ', '.join(arg_list_code))
         return result
@@ -7281,7 +7306,7 @@ class AttributeNode(ExprNode):
                 code.putln(
                     "%s = %s;" % (
                         select_code,
-                        rhs.result_as(self.ctype())))
+                        rhs.move_result_rhs_as(self.ctype())))
                         #rhs.result()))
             rhs.generate_post_assignment_code(code)
             rhs.free_temps(code)
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index 73baa9ee0..3f97d149f 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -3351,7 +3351,13 @@ class DefNodeWrapper(FuncDefNode):
         if self.signature.has_dummy_arg:
             args.append(Naming.self_cname)
         for arg in self.args:
-            if arg.hdr_type and not (arg.type.is_memoryviewslice or
+            if arg.hdr_type and arg.type.is_cpp_class:
+                # it's safe to move converted C++ types because they aren't
+                # used again afterwards
+                code.globalstate.use_utility_code(
+                    UtilityCode.load_cached("MoveIfSupported", "CppSupport.cpp"))
+                args.append("__PYX_STD_MOVE_IF_SUPPORTED(%s)" % arg.entry.cname)
+            elif arg.hdr_type and not (arg.type.is_memoryviewslice or
                                      arg.type.is_struct or
                                      arg.type.is_complex):
                 args.append(arg.type.cast_code(arg.entry.cname))
diff --git a/Cython/Utility/CppSupport.cpp b/Cython/Utility/CppSupport.cpp
index d086d848d..b31e2f268 100644
--- a/Cython/Utility/CppSupport.cpp
+++ b/Cython/Utility/CppSupport.cpp
@@ -58,3 +58,12 @@ auto __Pyx_pythran_to_python(T &&value) -> decltype(to_python(
 }
 
 #define __Pyx_PythranShapeAccessor(x) (x.shape().array())
+
+////////////// MoveIfSupported.proto //////////////////
+
+#if __cplusplus >= 201103L
+  #include <utility>
+  #define __PYX_STD_MOVE_IF_SUPPORTED(x) std::move(x)
+#else
+  #define __PYX_STD_MOVE_IF_SUPPORTED(x) x
+#endif
diff --git a/tests/compile/cpp_temp_assignment.pyx b/tests/compile/cpp_temp_assignment.pyx
new file mode 100644
index 000000000..40a2cc671
--- /dev/null
+++ b/tests/compile/cpp_temp_assignment.pyx
@@ -0,0 +1,65 @@
+# tag: cpp
+# mode: compile
+
+cdef extern from *:
+    """
+    #if __cplusplus >= 201103L
+    class NoAssign {
+        public:
+            NoAssign() {}
+            NoAssign(NoAssign&) = delete;
+            NoAssign(NoAssign&&) {}
+            NoAssign& operator=(NoAssign&) = delete;
+            NoAssign& operator=(NoAssign&&) { return *this; }
+            void func() {}
+    };
+    #else
+    // the test becomes meaningless
+    // (but just declare something to ensure it passes)
+    class NoAssign {
+        public:
+            void func() {}
+    };
+    #endif
+
+    NoAssign get_NoAssign_Py() {
+        return NoAssign();
+    }
+    NoAssign get_NoAssign_Cpp() {
+        return NoAssign();
+    }
+    """
+    cdef cppclass NoAssign:
+        void func()
+
+    # might raise Python exception (thus needs a temp)
+    NoAssign get_NoAssign_Py() except *
+    # might raise C++ exception (thus needs a temp)
+    NoAssign get_NoAssign_Cpp() except +
+
+cdef internal_cpp_func(NoAssign arg):
+    pass
+
+def test_call_to_function():
+    # will fail to compile if move constructors aren't used
+    internal_cpp_func(get_NoAssign_Py())
+    internal_cpp_func(get_NoAssign_Cpp())
+
+def test_assignment_to_name():
+    # will fail if move constructors aren't used
+    cdef NoAssign value
+    value = get_NoAssign_Py()
+    value = get_NoAssign_Cpp()
+
+def test_assignment_to_scope():
+    cdef NoAssign value
+    value = get_NoAssign_Py()
+    value = get_NoAssign_Cpp()
+    def inner():
+        value.func()
+
+cdef class AssignToClassAttr:
+    cdef NoAssign attr
+    def __init__(self):
+        self.attr = get_NoAssign_Py()
+        self.attr = get_NoAssign_Cpp()