Skip to content

Commit daab4c1

Browse files
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.
1 parent ee42d89 commit daab4c1

File tree

5 files changed

+41
-34
lines changed

5 files changed

+41
-34
lines changed

dynd/include/callables/assign_to_pyobject_callable.hpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ namespace nd {
294294
const dynd::string &rawname = src0_tp.extended<dynd::ndt::struct_type>()->get_field_name(i);
295295
pydynd::py_ref name =
296296
capture_if_not_null(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
297-
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, name.release());
297+
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, pydynd::release(std::move(name)));
298298
}
299299
self_ck->m_copy_el_offsets.resize(field_count);
300300

@@ -339,7 +339,7 @@ namespace nd {
339339
const dynd::string &rawname = src_tp[0].extended<dynd::ndt::struct_type>()->get_field_name(i);
340340
pydynd::py_ref name =
341341
capture_if_not_null(PyUnicode_DecodeUTF8(rawname.begin(), rawname.end() - rawname.begin(), NULL));
342-
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, name.release());
342+
PyTuple_SET_ITEM(self_ck->m_field_names.get(), i, pydynd::release(std::move(name)));
343343
}
344344
self_ck->m_copy_el_offsets.resize(field_count);
345345
for (intptr_t i = 0; i < field_count; ++i) {

dynd/include/kernels/assign_to_pyobject_kernel.hpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ struct assign_to_pyobject_kernel<ndt::tuple_type>
431431
if (PyErr_Occurred()) {
432432
throw std::exception();
433433
}
434-
*dst_obj = tup.release();
434+
*dst_obj = pydynd::release(std::move(tup));
435435
}
436436
};
437437

@@ -470,7 +470,7 @@ struct assign_to_pyobject_kernel<ndt::struct_type>
470470
if (PyErr_Occurred()) {
471471
throw std::exception();
472472
}
473-
*dst_obj = dct.release();
473+
*dst_obj = pydynd::release(std::move(dct));
474474
}
475475
};
476476

@@ -496,7 +496,7 @@ struct assign_to_pyobject_kernel<ndt::fixed_dim_type>
496496
if (PyErr_Occurred()) {
497497
throw std::exception();
498498
}
499-
*dst_obj = lst.release();
499+
*dst_obj = pydynd::release(std::move(lst));
500500
}
501501
};
502502

@@ -524,6 +524,6 @@ struct assign_to_pyobject_kernel<ndt::var_dim_type>
524524
if (PyErr_Occurred()) {
525525
throw std::exception();
526526
}
527-
*dst_obj = lst.release();
527+
*dst_obj = pydynd::release(std::move(lst));
528528
}
529529
};

dynd/include/utility_functions.hpp

+15-9
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ class py_ref_tmpl {
217217
// If this type is a non-null type, the assigned value should not be null.
218218
decref_if_owned<!owns_ref, not_null>(o);
219219
// If the reference is not null, assert that it is still valid.
220-
PYDYND_ASSERT_IF(o != nullptr, Py_REFCNT(o) > 0)
220+
PYDYND_ASSERT_IF(o != nullptr, Py_REFCNT(o) > 0);
221221
}
222222

223223
/* If:
@@ -384,22 +384,22 @@ class py_ref_tmpl {
384384
}
385385
}
386386

387+
py_ref_tmpl<false, not_null> borrow() noexcept { return py_ref<false, not_null>(o, false); }
388+
387389
// Return a reference to the encapsulated PyObject as a raw pointer.
388390
// Set the encapsulated pointer to NULL.
389391
// If this is a type that owns its reference, an owned reference is returned.
390392
// If this is a type that wraps a borrowed reference, a borrowed reference is returned.
391-
PyObject *release() noexcept
393+
static PyObject *release(py_ref_tmpl<owns_ref, not_null> &&ref) noexcept
392394
{
393395
// If the contained reference should not be null, assert that it isn't.
394-
PYDYND_ASSERT_IF(not_null, o != nullptr);
396+
PYDYND_ASSERT_IF(not_null, ref.o != nullptr);
395397
// If the contained reference is not null, assert that it is valid.
396-
PYDYND_ASSERT_IF(o != nullptr, Py_REFCNT(o) > 0);
397-
auto ret = o;
398-
o = nullptr;
398+
PYDYND_ASSERT_IF(ref.o != nullptr, Py_REFCNT(ref.o) > 0);
399+
PyObject *ret = ref.o;
400+
ref.o = nullptr;
399401
return ret;
400402
}
401-
402-
py_ref_tmpl<false, not_null> borrow() noexcept { return py_ref<false, not_null>(o, false); }
403403
};
404404

405405
// Convenience aliases for the templated smart pointer classes.
@@ -412,6 +412,12 @@ using py_borref = py_ref_tmpl<false, true>;
412412

413413
using py_borref_with_null = py_ref_tmpl<false, false>;
414414

415+
template <bool owns_ref, bool not_null>
416+
PyObject *release(py_ref_tmpl<owns_ref, not_null> &&ref)
417+
{
418+
return py_ref_tmpl<owns_ref, not_null>::release(std::forward<py_ref_tmpl<owns_ref, not_null>>(ref));
419+
}
420+
415421
/* Capture a new reference if it is not null.
416422
* Throw an exception if it is.
417423
*/
@@ -441,7 +447,7 @@ inline py_ref_tmpl<owns_ref, true> nullcheck(py_ref_tmpl<owns_ref, not_null> &&o
441447
* This should be used when the pointer is already known to not be null.
442448
*/
443449
template <bool owns_ref, bool not_null>
444-
inline py_ref_tmpl<owns_ref, true> nullcheck(py_ref_tmpl<owns_ref, not_null> &&obj) noexcept
450+
inline py_ref_tmpl<owns_ref, true> disallow_null(py_ref_tmpl<owns_ref, not_null> &&obj) noexcept
445451
{
446452
// Assert that the wrapped pointer is actually not null.
447453
assert(obj.get() != nullptr);

dynd/src/array_as_numpy.cpp

+16-15
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,8 @@ static void make_numpy_dtype_for_copy(py_ref *out_numpy_dtype, intptr_t ndim, co
145145
make_numpy_dtype_for_copy(&child_numpy_dtype, 0, element_tp, arrmeta);
146146
// Create the result numpy dtype
147147
py_ref tuple_obj = capture_if_not_null(PyTuple_New(2));
148-
PyTuple_SET_ITEM(tuple_obj.get(), 0, child_numpy_dtype.release());
149-
PyTuple_SET_ITEM(tuple_obj.get(), 1, shape.release());
148+
PyTuple_SET_ITEM(tuple_obj.get(), 0, release(std::move(child_numpy_dtype)));
149+
PyTuple_SET_ITEM(tuple_obj.get(), 1, release(std::move(shape)));
150150

151151
PyArray_Descr *result = NULL;
152152
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
170170
#else
171171
py_ref name_str = capture_if_not_null(PyString_FromStringAndSize(fn.begin(), fn.end() - fn.begin()));
172172
#endif
173-
PyList_SET_ITEM(names_obj.get(), i, name_str.release());
173+
PyList_SET_ITEM(names_obj.get(), i, release(std::move(name_str)));
174174
}
175175

176176
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
184184
size_t field_size = ((PyArray_Descr *)field_numpy_dtype.get())->elsize;
185185
standard_offset = inc_to_alignment(standard_offset, field_alignment);
186186
standard_alignment = max(standard_alignment, field_alignment);
187-
PyList_SET_ITEM(formats_obj.get(), i, field_numpy_dtype.release());
187+
PyList_SET_ITEM(formats_obj.get(), i, release(std::move(field_numpy_dtype)));
188188
PyList_SET_ITEM(offsets_obj.get(), i, PyLong_FromSize_t(standard_offset));
189189
standard_offset += field_size;
190190
}
@@ -333,8 +333,8 @@ static void as_numpy_analysis(py_ref *out_numpy_dtype, bool *out_requires_copy,
333333
}
334334
// Create the result numpy dtype
335335
py_ref tuple_obj = capture_if_not_null(PyTuple_New(2));
336-
PyTuple_SET_ITEM(tuple_obj.get(), 0, child_numpy_dtype.release());
337-
PyTuple_SET_ITEM(tuple_obj.get(), 1, shape.release());
336+
PyTuple_SET_ITEM(tuple_obj.get(), 0, release(std::move(child_numpy_dtype)));
337+
PyTuple_SET_ITEM(tuple_obj.get(), 1, release(std::move(shape)));
338338
339339
PyArray_Descr *result = NULL;
340340
if (!PyArray_DescrConverter(tuple_obj, &result)) {
@@ -367,7 +367,7 @@ static void as_numpy_analysis(py_ref *out_numpy_dtype, bool *out_requires_copy,
367367
#else
368368
py_ref name_str = capture_if_not_null(PyString_FromStringAndSize(fn.begin(), fn.end() - fn.begin()));
369369
#endif
370-
PyList_SET_ITEM(names_obj.get(), i, name_str.release());
370+
PyList_SET_ITEM(names_obj.get(), i, release(std::move(name_str)));
371371
}
372372

373373
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,
380380
*out_numpy_dtype = py_ref(Py_None, false);
381381
return;
382382
}
383-
PyList_SET_ITEM(formats_obj.get(), i, field_numpy_dtype.release());
383+
PyList_SET_ITEM(formats_obj.get(), i, release(std::move(field_numpy_dtype)));
384384
}
385385

386386
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)
528528
throw dynd::type_error(ss.str());
529529
}
530530
}
531-
return result.release();
531+
return release(std::move(result));
532532
}
533533

534534
if (a.get_type().get_id() == var_dim_id) {
@@ -571,18 +571,19 @@ PyObject *pydynd::array_as_numpy(PyObject *a_obj, bool allow_copy)
571571
}
572572

573573
// Create a new NumPy array, and copy from the dynd array
574-
py_ref result = capture_if_not_null(PyArray_NewFromDescr(&PyArray_Type, (PyArray_Descr *)numpy_dtype.release(),
575-
(int)ndim, shape.get(), strides.get(), NULL, 0, NULL));
574+
py_ref result = capture_if_not_null(
575+
PyArray_NewFromDescr(&PyArray_Type, reinterpret_cast<PyArray_Descr *>(release(std::move(numpy_dtype))),
576+
(int)ndim, shape.get(), strides.get(), NULL, 0, NULL));
576577
array_copy_to_numpy((PyArrayObject *)result.get(), a.get_type(), a.get()->metadata(), a.cdata());
577578

578579
// Return the NumPy array
579-
return result.release();
580+
return release(std::move(result));
580581
}
581582
else {
582583
// Create a view directly to the dynd array
583584
py_ref result = capture_if_not_null(PyArray_NewFromDescr(
584-
&PyArray_Type, (PyArray_Descr *)numpy_dtype.release(), (int)ndim, shape.get(), strides.get(),
585-
const_cast<char *>(a.cdata()),
585+
&PyArray_Type, reinterpret_cast<PyArray_Descr *>(release(std::move(numpy_dtype))), (int)ndim, shape.get(),
586+
strides.get(), const_cast<char *>(a.cdata()),
586587
((a.get_flags() & nd::write_access_flag) ? NPY_ARRAY_WRITEABLE : 0) | NPY_ARRAY_ALIGNED, NULL));
587588

588589
#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)
594595
PyArray_BASE(result.get()) = n_obj;
595596
Py_INCREF(n_obj);
596597
#endif
597-
return result.release();
598+
return release(std::move(result));
598599
}
599600
}
600601

dynd/src/numpy_type_interop.cpp

+4-4
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ ttp->get_field_types_raw(), offsets.data());
116116
py_ref name_str = capture_if_not_null(PyString_FromStringAndSize(
117117
fname.begin, fname.end - fname.begin));
118118
#endif
119-
PyList_SET_ITEM(names_obj.get(), i, name_str.release());
119+
PyList_SET_ITEM(names_obj.get(), i, release(std::move(name_str)));
120120
}
121121
py_ref formats_obj = capture_if_not_null(PyList_New(field_count));
122122
for (size_t i = 0; i < field_count; ++i) {
@@ -175,8 +175,8 @@ dtype because it is not C-order";
175175
py_ref shape_obj = capture_if_not_null(intptr_array_as_tuple((int)shape.size(),
176176
&shape[0]));
177177
py_ref tuple_obj = capture_if_not_null(PyTuple_New(2));
178-
PyTuple_SET_ITEM(tuple_obj.get(), 0, dtype_obj.release());
179-
PyTuple_SET_ITEM(tuple_obj.get(), 1, shape_obj.release());
178+
PyTuple_SET_ITEM(tuple_obj.get(), 0, release(std::move(dtype_obj)));
179+
PyTuple_SET_ITEM(tuple_obj.get(), 1, release(std::move(shape_obj)));
180180
PyArray_Descr *result = NULL;
181181
if (PyArray_DescrConverter(tuple_obj, &result) != NPY_SUCCEED) {
182182
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
218218
#else
219219
py_ref name_str = capture_if_not_null(PyString_FromStringAndSize(fname.begin(), fname.end() - fname.begin()));
220220
#endif
221-
PyList_SET_ITEM(names_obj.get(), i, name_str.release());
221+
PyList_SET_ITEM(names_obj.get(), i, release(std::move(name_str)));
222222
}
223223

224224
py_ref formats_obj = capture_if_not_null(PyList_New(field_count));

0 commit comments

Comments
 (0)