Move the release method of pydynd::py_ref_tmpl to be a function that …

…takes an rvalue reference so that its inputs are explicitly invalidated when release is used.
libdynd · insertinterestingnamehere · May 24, 2016 · May 24, 2016 · Jun 3, 2016 · Jun 4, 2016
commit 059e012ba7b5fb3e1af378678aae8909ed469800
diff --git a/dynd/include/callables/assign_to_pyobject_callable.hpp b/dynd/include/callables/assign_to_pyobject_callable.hpp
@@ -294,7 +294,7 @@ namespace nd {
           const dynd::string &rawname = src0_tp.extended<dynd::ndt::struct_type>()->get_field_name(i);
           pydynd::py_ref name =
               capture_if_not_null(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
-          PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, name.release());
+          PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, pydynd::release(std::move(name)));
         }
         self_ck->m_copy_el_offsets.resize(field_count);
 
@@ -339,7 +339,7 @@ namespace nd {
             const dynd::string &rawname = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_name(i);
             pydynd::py_ref name =
                 capture_if_not_null(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
-            PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, name.release());
+            PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, pydynd::release(std::move(name)));
           }
           self_ck->m_copy_el_offsets.resize(field_count);
           for (intptr_t i = 0; i < field_count; ++i) {

diff --git a/dynd/include/kernels/assign_to_pyobject_kernel.hpp b/dynd/include/kernels/assign_to_pyobject_kernel.hpp
@@ -431,7 +431,7 @@ struct assign_to_pyobject_kernel<ndt::tuple_type>
     if (PyErr_Occurred()) {
       throw std::exception();
     }
-    *dst_obj = tup.release();
+    *dst_obj = pydynd::release(std::move(tup));
   }
 };
 
@@ -470,7 +470,7 @@ struct assign_to_pyobject_kernel<ndt::struct_type>
     if (PyErr_Occurred()) {
       throw std::exception();
     }
-    *dst_obj = dct.release();
+    *dst_obj = pydynd::release(std::move(dct));
   }
 };
 
@@ -496,7 +496,7 @@ struct assign_to_pyobject_kernel<ndt::fixed_dim_type>
     if (PyErr_Occurred()) {
       throw std::exception();
     }
-    *dst_obj = lst.release();
+    *dst_obj = pydynd::release(std::move(lst));
   }
 };
 
@@ -524,6 +524,6 @@ struct assign_to_pyobject_kernel<ndt::var_dim_type>
     if (PyErr_Occurred()) {
       throw std::exception();
     }
-    *dst_obj = lst.release();
+    *dst_obj = pydynd::release(std::move(lst));
   }
 };
diff --git a/dynd/include/utility_functions.hpp b/dynd/include/utility_functions.hpp
@@ -217,7 +217,7 @@ class py_ref_tmpl {
     // If this type is a non-null type, the assigned value should not be null.
     decref_if_owned<!owns_ref, not_null>(o);
     // If the reference is not null, assert that it is still valid.
-    PYDYND_ASSERT_IF(o != nullptr, Py_REFCNT(o) > 0)
+    PYDYND_ASSERT_IF(o != nullptr, Py_REFCNT(o) > 0);
   }
 
   /* If:
@@ -384,22 +384,22 @@ class py_ref_tmpl {
     }
   }
 
+  py_ref_tmpl<false, not_null> borrow() noexcept { return py_ref<false, not_null>(o, false); }
+
   // Return a reference to the encapsulated PyObject as a raw pointer.
   // Set the encapsulated pointer to NULL.
   // If this is a type that owns its reference, an owned reference is returned.
   // If this is a type that wraps a borrowed reference, a borrowed reference is returned.
-  PyObject *release() noexcept
+  static PyObject *release(py_ref_tmpl<owns_ref, not_null> &&ref) noexcept
   {
     // If the contained reference should not be null, assert that it isn't.
-    PYDYND_ASSERT_IF(not_null, o != nullptr);
+    PYDYND_ASSERT_IF(not_null, ref.o != nullptr);
     // If the contained reference is not null, assert that it is valid.
-    PYDYND_ASSERT_IF(o != nullptr, Py_REFCNT(o) > 0);
-    auto ret = o;
-    o = nullptr;
+    PYDYND_ASSERT_IF(ref.o != nullptr, Py_REFCNT(ref.o) > 0);
+    PyObject *ret = ref.o;
+    ref.o = nullptr;
     return ret;
   }
-
-  py_ref_tmpl<false, not_null> borrow() noexcept { return py_ref<false, not_null>(o, false); }
 };
 
 // Convenience aliases for the templated smart pointer classes.
@@ -412,6 +412,12 @@ using py_borref = py_ref_tmpl<false, true>;
 
 using py_borref_with_null = py_ref_tmpl<false, false>;
 
+template <bool owns_ref, bool not_null>
+PyObject *release(py_ref_tmpl<owns_ref, not_null> &&ref)
+{
+  return py_ref_tmpl<owns_ref, not_null>::release(std::forward<py_ref_tmpl<owns_ref, not_null>>(ref));
+}
+
 /* Capture a new reference if it is not null.
  * Throw an exception if it is.
  */

diff --git a/dynd/src/array_as_numpy.cpp b/dynd/src/array_as_numpy.cpp
@@ -145,8 +145,8 @@ static void make_numpy_dtype_for_copy(py_ref *out_numpy_dtype, intptr_t ndim, co
       make_numpy_dtype_for_copy(&child_numpy_dtype, 0, element_tp, arrmeta);
       // Create the result numpy dtype
       py_ref tuple_obj = capture_if_not_null(PyTuple_New(2));
-      PyTuple_SET_ITEM(tuple_obj.get(), 0, child_numpy_dtype.release());
-      PyTuple_SET_ITEM(tuple_obj.get(), 1, shape.release());
+      PyTuple_SET_ITEM(tuple_obj.get(), 0, release(std::move(child_numpy_dtype)));
+      PyTuple_SET_ITEM(tuple_obj.get(), 1, release(std::move(shape)));
 
       PyArray_Descr *result = NULL;
       if (!PyArray_DescrConverter(tuple_obj.get(), &result)) {
@@ -170,7 +170,7 @@ static void make_numpy_dtype_for_copy(py_ref *out_numpy_dtype, intptr_t ndim, co
 #else
       py_ref name_str = capture_if_not_null(PyString_FromStringAndSize(fn.begin(), fn.end() - fn.begin()));
 #endif
-      PyList_SET_ITEM(names_obj.get(), i, name_str.release());
+      PyList_SET_ITEM(names_obj.get(), i, release(std::move(name_str)));
     }
 
     py_ref formats_obj = capture_if_not_null(PyList_New(field_count));
@@ -184,7 +184,7 @@ static void make_numpy_dtype_for_copy(py_ref *out_numpy_dtype, intptr_t ndim, co
       size_t field_size = ((PyArray_Descr *)field_numpy_dtype.get())->elsize;
       standard_offset = inc_to_alignment(standard_offset, field_alignment);
       standard_alignment = max(standard_alignment, field_alignment);
-      PyList_SET_ITEM(formats_obj.get(), i, field_numpy_dtype.release());
+      PyList_SET_ITEM(formats_obj.get(), i, release(std::move(field_numpy_dtype)));
       PyList_SET_ITEM(offsets_obj.get(), i, PyLong_FromSize_t(standard_offset));
       standard_offset += field_size;
     }
@@ -333,8 +333,8 @@ static void as_numpy_analysis(py_ref *out_numpy_dtype, bool *out_requires_copy,
         }
         // Create the result numpy dtype
         py_ref tuple_obj = capture_if_not_null(PyTuple_New(2));
-        PyTuple_SET_ITEM(tuple_obj.get(), 0, child_numpy_dtype.release());
-        PyTuple_SET_ITEM(tuple_obj.get(), 1, shape.release());
+        PyTuple_SET_ITEM(tuple_obj.get(), 0, release(std::move(child_numpy_dtype)));
+        PyTuple_SET_ITEM(tuple_obj.get(), 1, release(std::move(shape)));
 
         PyArray_Descr *result = NULL;
         if (!PyArray_DescrConverter(tuple_obj, &result)) {
@@ -367,7 +367,7 @@ static void as_numpy_analysis(py_ref *out_numpy_dtype, bool *out_requires_copy,
 #else
       py_ref name_str = capture_if_not_null(PyString_FromStringAndSize(fn.begin(), fn.end() - fn.begin()));
 #endif
-      PyList_SET_ITEM(names_obj.get(), i, name_str.release());
+      PyList_SET_ITEM(names_obj.get(), i, release(std::move(name_str)));
     }
 
     py_ref formats_obj = capture_if_not_null(PyList_New(field_count));
@@ -380,7 +380,7 @@ static void as_numpy_analysis(py_ref *out_numpy_dtype, bool *out_requires_copy,
         *out_numpy_dtype = py_ref(Py_None, false);
         return;
       }
-      PyList_SET_ITEM(formats_obj.get(), i, field_numpy_dtype.release());
+      PyList_SET_ITEM(formats_obj.get(), i, release(std::move(field_numpy_dtype)));
     }
 
     py_ref offsets_obj = capture_if_not_null(PyList_New(field_count));
@@ -528,7 +528,7 @@ PyObject *pydynd::array_as_numpy(PyObject *a_obj, bool allow_copy)
       throw dynd::type_error(ss.str());
     }
     }
-    return result.release();
+    return release(std::move(result));
   }
 
   if (a.get_type().get_id() == var_dim_id) {
@@ -571,18 +571,19 @@ PyObject *pydynd::array_as_numpy(PyObject *a_obj, bool allow_copy)
     }
 
     // Create a new NumPy array, and copy from the dynd array
-    py_ref result = capture_if_not_null(PyArray_NewFromDescr(&PyArray_Type, (PyArray_Descr *)numpy_dtype.release(),
-                                                             (int)ndim, shape.get(), strides.get(), NULL, 0, NULL));
+    py_ref result = capture_if_not_null(
+        PyArray_NewFromDescr(&PyArray_Type, reinterpret_cast<PyArray_Descr *>(release(std::move(numpy_dtype))),
+                             (int)ndim, shape.get(), strides.get(), NULL, 0, NULL));
     array_copy_to_numpy((PyArrayObject *)result.get(), a.get_type(), a.get()->metadata(), a.cdata());
 
     // Return the NumPy array
-    return result.release();
+    return release(std::move(result));
   }
   else {
     // Create a view directly to the dynd array
     py_ref result = capture_if_not_null(PyArray_NewFromDescr(
-        &PyArray_Type, (PyArray_Descr *)numpy_dtype.release(), (int)ndim, shape.get(), strides.get(),
-        const_cast<char *>(a.cdata()),
+        &PyArray_Type, reinterpret_cast<PyArray_Descr *>(release(std::move(numpy_dtype))), (int)ndim, shape.get(),
+        strides.get(), const_cast<char *>(a.cdata()),
         ((a.get_flags() & nd::write_access_flag) ? NPY_ARRAY_WRITEABLE : 0) | NPY_ARRAY_ALIGNED, NULL));
 
 #if NPY_API_VERSION >= 7 // At least NumPy 1.7
@@ -594,7 +595,7 @@ PyObject *pydynd::array_as_numpy(PyObject *a_obj, bool allow_copy)
     PyArray_BASE(result.get()) = n_obj;
     Py_INCREF(n_obj);
 #endif
-    return result.release();
+    return release(std::move(result));
   }
 }
 

diff --git a/dynd/src/numpy_type_interop.cpp b/dynd/src/numpy_type_interop.cpp
@@ -116,7 +116,7 @@ ttp->get_field_types_raw(), offsets.data());
                 py_ref name_str = capture_if_not_null(PyString_FromStringAndSize(
                     fname.begin, fname.end - fname.begin));
 #endif
-                PyList_SET_ITEM(names_obj.get(), i, name_str.release());
+                PyList_SET_ITEM(names_obj.get(), i, release(std::move(name_str)));
             }
             py_ref formats_obj = capture_if_not_null(PyList_New(field_count));
             for (size_t i = 0; i < field_count; ++i) {
@@ -175,8 +175,8 @@ dtype because it is not C-order";
             py_ref shape_obj = capture_if_not_null(intptr_array_as_tuple((int)shape.size(),
 &shape[0]));
             py_ref tuple_obj = capture_if_not_null(PyTuple_New(2));
-            PyTuple_SET_ITEM(tuple_obj.get(), 0, dtype_obj.release());
-            PyTuple_SET_ITEM(tuple_obj.get(), 1, shape_obj.release());
+            PyTuple_SET_ITEM(tuple_obj.get(), 0, release(std::move(dtype_obj)));
+            PyTuple_SET_ITEM(tuple_obj.get(), 1, release(std::move(shape_obj)));
             PyArray_Descr *result = NULL;
             if (PyArray_DescrConverter(tuple_obj, &result) != NPY_SUCCEED) {
                 throw dynd::type_error("failed to convert dynd type into numpy
@@ -218,7 +218,7 @@ PyArray_Descr *pydynd::numpy_dtype_from__type(const dynd::ndt::type &tp, const c
 #else
       py_ref name_str = capture_if_not_null(PyString_FromStringAndSize(fname.begin(), fname.end() - fname.begin()));
 #endif
-      PyList_SET_ITEM(names_obj.get(), i, name_str.release());
+      PyList_SET_ITEM(names_obj.get(), i, release(std::move(name_str)));
     }
 
     py_ref formats_obj = capture_if_not_null(PyList_New(field_count));