Skip to content

Commit 3dcb817

Browse files
committed
Be sure to properly treat both rvalues and lvalues in c2py functions
1 parent 7d5195d commit 3dcb817

File tree

11 files changed

+103
-56
lines changed

11 files changed

+103
-56
lines changed

c++/cpp2py/converters/map.hpp

+15-4
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,28 @@
11
#pragma once
22
#include <map>
3+
#include "../traits.hpp"
34

45
namespace cpp2py {
56

67
template <typename K, typename V> struct py_converter<std::map<K, V>> {
78

8-
static PyObject *c2py(std::map<K, V> const &m) {
9+
template <typename M> static PyObject *c2py(M &&m) {
10+
static_assert(is_instantiation_of_v<std::map, std::decay_t<M>>, "Logic Error");
11+
912
PyObject *d = PyDict_New();
10-
for (auto &x : m) {
11-
pyref k = py_converter<K>::c2py(x.first);
13+
for (auto &[key, val] : m) {
14+
pyref k, v;
15+
if constexpr (std::is_reference_v<M>) {
16+
k = convert_to_python(key);
17+
v = convert_to_python(val);
18+
} else { // Map passed as rvalue
19+
k = convert_to_python(std::move(key));
20+
v = convert_to_python(std::move(val));
21+
}
22+
1223
// if the K is a list, we transform into a tuple
1324
if (PyList_Check(k)) k = PyList_AsTuple(k);
14-
pyref v = py_converter<V>::c2py(x.second);
25+
1526
if (k.is_null() or v.is_null() or (PyDict_SetItem(d, k, v) == -1)) {
1627
Py_DECREF(d);
1728
return NULL;

c++/cpp2py/converters/optional.hpp

+8-5
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
11
#pragma once
2+
#include "../traits.hpp"
3+
24
namespace cpp2py {
35

46
template <typename T> struct py_converter<std::optional<T>> {
57

6-
using conv = py_converter<T>;
8+
using conv = py_converter<std::decay_t<T>>;
79

8-
static PyObject *c2py(std::optional<T> &op) {
9-
if (!bool(op)) Py_RETURN_NONE;
10-
return conv::c2py(*op);
11-
}
10+
template <typename O> static PyObject *c2py(O &&op) {
11+
static_assert(is_instantiation_of_v<std::optional, std::decay_t<O>>, "Logic Error");
12+
if (!bool(op)) Py_RETURN_NONE;
13+
return conv::c2py(*(std::forward<O>(op)));
14+
}
1215

1316
static bool is_convertible(PyObject *ob, bool raise_exception) {
1417
return ((ob == Py_None) or conv::is_convertible(ob, raise_exception));

c++/cpp2py/converters/pair.hpp

+6-4
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
#pragma once
2-
//#include <utility>
2+
#include "../traits.hpp"
33

44
namespace cpp2py {
55

66
template <typename T1, typename T2> struct py_converter<std::pair<T1, T2>> {
77

8-
static PyObject *c2py(std::pair<T1, T2> const &p) {
9-
pyref x1 = py_converter<T1>::c2py(std::get<0>(p));
10-
pyref x2 = py_converter<T2>::c2py(std::get<1>(p));
8+
template <typename P> static PyObject *c2py(P &&p) {
9+
static_assert(is_instantiation_of_v<std::pair, std::decay_t<P>>, "Logic error");
10+
pyref x1 = convert_to_python(std::get<0>(std::forward<P>(p)));
11+
pyref x2 = convert_to_python(std::get<1>(std::forward<P>(p)));
12+
1113
if (x1.is_null() or x2.is_null()) return NULL;
1214
return PyTuple_Pack(2, (PyObject *)x1, (PyObject *)x2);
1315
}

c++/cpp2py/converters/set.hpp

+10-3
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,21 @@
11
#pragma once
22
#include <set>
3+
#include "../traits.hpp"
34

45
namespace cpp2py {
56

67
template <typename K> struct py_converter<std::set<K>> {
78

8-
static PyObject *c2py(std::set<K> const &s) {
9+
template <typename S> static PyObject *c2py(S &&s) {
10+
static_assert(is_instantiation_of_v<std::set, std::decay_t<S>>, "Logic error");
911
PyObject *set = PySet_New(NULL);
1012
for (auto &x : s) {
11-
pyref y = py_converter<K>::c2py(x);
13+
pyref y;
14+
if constexpr (std::is_reference_v<S>) {
15+
y = convert_to_python(x);
16+
} else { // s passed as rvalue
17+
y = convert_to_python(std::move(x));
18+
}
1219
if (y.is_null() or (PySet_Add(set, y) == -1)) {
1320
Py_DECREF(set);
1421
return NULL;
@@ -31,7 +38,7 @@ namespace cpp2py {
3138
if (raise_exception) { PyErr_SetString(PyExc_TypeError, "Cannot convert to std::set"); }
3239
return false;
3340
}
34-
41+
3542
static std::set<K> py2c(PyObject *ob) {
3643
std::set<K> res;
3744
pyref keys_it = PyObject_GetIter(ob);

c++/cpp2py/converters/tuple.hpp

+14-11
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
#include <tuple>
44
#include <numeric>
55

6+
#include "../traits.hpp"
7+
68
namespace cpp2py {
79

810
template <typename... Types> struct py_converter<std::tuple<Types...>> {
@@ -11,28 +13,29 @@ namespace cpp2py {
1113
using tuple_t = std::tuple<Types...>;
1214

1315
// c2py implementation
14-
template <std::size_t... Is> static PyObject *c2py_impl(tuple_t const &t, std::index_sequence<Is...>) {
15-
auto objs = std::array<pyref, sizeof...(Is)>{pyref(py_converter<Types>::c2py(std::get<Is>(t)))...};
16+
template <typename T, auto... Is> static PyObject *c2py_impl(T &&t, std::index_sequence<Is...>) {
17+
auto objs = std::array<pyref, sizeof...(Is)>{convert_to_python(std::get<Is>(std::forward<T>(t)))...};
1618
bool one_is_null = std::accumulate(std::begin(objs), std::end(objs), false, [](bool x, PyObject *a) { return x or (a == NULL); });
1719
if (one_is_null) return NULL;
1820
return PyTuple_Pack(sizeof...(Types), (PyObject *)(objs[Is])...);
1921
}
2022

2123
// is_convertible implementation
22-
template <int N, typename T, typename... Tail> static bool is_convertible_impl(PyObject *seq, bool raise_exception) {
23-
return py_converter<T>::is_convertible(PySequence_Fast_GET_ITEM(seq, N), raise_exception)
24-
&& is_convertible_impl<N + 1, Tail...>(seq, raise_exception);
24+
template <auto... Is> static bool is_convertible_impl(PyObject *seq, bool raise_exception, std::index_sequence<Is...>) {
25+
return (py_converter<std::decay_t<Types>>::is_convertible(PySequence_Fast_GET_ITEM(seq, Is), raise_exception) and ...);
2526
}
26-
template <int> static bool is_convertible_impl(PyObject *seq, bool raise_exception) { return true; }
2727

28-
template <size_t... Is> static auto py2c_impl(std::index_sequence<Is...>, PyObject *seq) {
29-
return std::make_tuple(py_converter<Types>::py2c(PySequence_Fast_GET_ITEM(seq, Is))...);
28+
template <auto... Is> static auto py2c_impl(PyObject *seq, std::index_sequence<Is...>) {
29+
return std::make_tuple(py_converter<std::decay_t<Types>>::py2c(PySequence_Fast_GET_ITEM(seq, Is))...);
3030
}
3131

3232
public:
3333
// -----------------------------------------
3434

35-
static PyObject *c2py(tuple_t const &t) { return c2py_impl(t, std::make_index_sequence<sizeof...(Types)>()); }
35+
template <typename T> static PyObject *c2py(T &&t) {
36+
static_assert(is_instantiation_of_v<std::tuple, std::decay_t<T>>, "Logic Error");
37+
return c2py_impl(std::forward<T>(t), std::make_index_sequence<sizeof...(Types)>());
38+
}
3639

3740
// -----------------------------------------
3841

@@ -41,7 +44,7 @@ namespace cpp2py {
4144
pyref seq = PySequence_Fast(ob, "expected a sequence");
4245
// Sizes must match! Could we relax this condition to '<'?
4346
if (PySequence_Fast_GET_SIZE((PyObject *)seq) != std::tuple_size<tuple_t>::value) goto _false;
44-
if (!is_convertible_impl<0, Types...>((PyObject *)seq, raise_exception)) goto _false;
47+
if (!is_convertible_impl((PyObject *)seq, raise_exception, std::make_index_sequence<sizeof...(Types)>())) goto _false;
4548
return true;
4649
}
4750
_false:
@@ -53,7 +56,7 @@ namespace cpp2py {
5356

5457
static tuple_t py2c(PyObject *ob) {
5558
pyref seq = PySequence_Fast(ob, "expected a sequence");
56-
return py2c_impl(std::make_index_sequence<sizeof...(Types)>(), (PyObject *)seq);
59+
return py2c_impl((PyObject *)seq, std::make_index_sequence<sizeof...(Types)>());
5760
}
5861
};
5962
} // namespace cpp2py

c++/cpp2py/converters/variant.hpp

+7-6
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
#include <tuple>
33
#include <variant>
44

5+
#include "../traits.hpp"
6+
57
namespace cpp2py {
68

79
// std::variant<T...> converter
@@ -45,18 +47,17 @@ namespace cpp2py {
4547
}
4648

4749
struct _visitor {
48-
template <typename U> PyObject *operator()(U const &x) { return py_converter<U>::c2py(x); }
50+
template <typename U> PyObject *operator()(U &&x) { return convert_to_python(std::forward<U>(x)); }
4951
};
5052

5153
public:
52-
static PyObject *c2py(std::variant<T...> const &v) {
53-
//auto l = [](auto const &x) -> PyObject * { return py_converter<decltype(x)>::c2py(x); };
54-
return visit(_visitor{}, v);
55-
//return visit(_visitor{}, v);
54+
template <typename V> static PyObject *c2py(V &&v) {
55+
static_assert(is_instantiation_of_v<std::variant, std::decay_t<V>>);
56+
return visit(_visitor{}, std::forward<V>(v));
5657
}
5758

5859
static bool is_convertible(PyObject *ob, bool raise_exception) {
59-
if ((... or py_converter<T>::is_convertible(ob, false))) return true;
60+
if ((... or py_converter<std::decay_t<T>>::is_convertible(ob, false))) return true;
6061
if (raise_exception) { PyErr_SetString(PyExc_TypeError, "Cannot convert to std::variant"); }
6162
return false;
6263
}

c++/cpp2py/converters/vector.hpp

+28-20
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,30 @@
33
#include <string>
44
#include <numpy/arrayobject.h>
55

6+
#include "../traits.hpp"
67
#include "../macros.hpp"
78
#include "../numpy_proxy.hpp"
89

910
namespace cpp2py {
1011

11-
template <typename T> static void delete_pycapsule(PyObject *capsule) {
12-
auto *ptr = static_cast<std::unique_ptr<T[]> *>(PyCapsule_GetPointer(capsule, "guard"));
13-
delete ptr;
14-
}
15-
1612
// Convert vector to numpy_proxy, WARNING: Deep Copy
17-
template <typename T> numpy_proxy make_numpy_proxy_from_vector(std::vector<T> const &v) {
13+
template <typename V> numpy_proxy make_numpy_proxy_from_vector(V &&v) {
14+
static_assert(is_instantiation_of_v<std::vector, std::decay_t<V>>, "Logic error");
15+
using value_type = typename std::remove_reference_t<V>::value_type;
1816

19-
auto *data_ptr = new std::unique_ptr<T[]>{new T[v.size()]};
20-
std::copy(begin(v), end(v), data_ptr->get());
21-
auto capsule = PyCapsule_New(data_ptr, "guard", &delete_pycapsule<T>);
17+
auto *vec_heap = new std::vector<value_type>{std::forward<V>(v)};
18+
auto delete_pycapsule = [](PyObject *capsule) {
19+
auto *ptr = static_cast<std::vector<value_type> *>(PyCapsule_GetPointer(capsule, "guard"));
20+
delete ptr;
21+
};
22+
PyObject *capsule = PyCapsule_New(vec_heap, "guard", delete_pycapsule);
2223

2324
return {1, // rank
24-
npy_type<std::remove_const_t<T>>,
25-
(void *)data_ptr->get(),
26-
std::is_const_v<T>,
27-
v_t{static_cast<long>(v.size())}, // extents
28-
v_t{sizeof(T)}, // strides
25+
npy_type<value_type>,
26+
(void *)vec_heap->data(),
27+
std::is_const_v<value_type>,
28+
std::vector<long>{long(vec_heap->size())}, // extents
29+
std::vector<long>{sizeof(value_type)}, // strides
2930
capsule};
3031
}
3132

@@ -50,14 +51,21 @@ namespace cpp2py {
5051

5152
template <typename T> struct py_converter<std::vector<T>> {
5253

53-
static PyObject *c2py(std::vector<T> const &v) {
54+
template <typename V> static PyObject *c2py(V &&v) {
55+
static_assert(is_instantiation_of_v<std::vector, std::decay_t<V>>, "Logic error");
56+
using value_type = typename std::remove_reference_t<V>::value_type;
5457

55-
if constexpr (has_npy_type<T>) {
56-
return make_numpy_proxy_from_vector(v).to_python();
58+
if constexpr (has_npy_type<value_type>) {
59+
return make_numpy_proxy_from_vector(std::forward<V>(v)).to_python();
5760
} else { // Convert to Python List
5861
PyObject *list = PyList_New(0);
59-
for (auto const &x : v) {
60-
pyref y = py_converter<T>::c2py(x);
62+
for (auto &x : v) {
63+
pyref y;
64+
if constexpr(std::is_reference_v<V>){
65+
y = py_converter<value_type>::c2py(x);
66+
} else { // Vector passed as rvalue
67+
y = py_converter<value_type>::c2py(std::move(x));
68+
}
6169
if (y.is_null() or (PyList_Append(list, y) == -1)) {
6270
Py_DECREF(list);
6371
return NULL;
@@ -122,7 +130,7 @@ namespace cpp2py {
122130
std::vector<T> res;
123131
pyref seq = PySequence_Fast(ob, "expected a sequence");
124132
int len = PySequence_Size(ob);
125-
for (int i = 0; i < len; i++) res.push_back(py_converter<T>::py2c(PySequence_Fast_GET_ITEM((PyObject *)seq, i))); //borrowed ref
133+
for (int i = 0; i < len; i++) res.push_back(py_converter<std::decay_t<T>>::py2c(PySequence_Fast_GET_ITEM((PyObject *)seq, i))); //borrowed ref
126134
return res;
127135
}
128136
};

c++/cpp2py/numpy_proxy.hpp

+1-3
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,13 @@
99

1010
namespace cpp2py {
1111

12-
using v_t = std::vector<long>;
13-
1412
// the basic information for a numpy array
1513
struct numpy_proxy {
1614
int rank = 0;
1715
long element_type = 0;
1816
void *data = nullptr;
1917
bool is_const = false;
20-
v_t extents, strides;
18+
std::vector<long> extents, strides;
2119
PyObject *base = nullptr; // The ref. counting guard typically
2220

2321
// Returns a new ref (or NULL if failure) with a new numpy.

c++/cpp2py/py_converter.hpp

+2
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ namespace cpp2py {
127127

128128
using is_wrapped_type = void; // to recognize
129129

130+
static_assert(not std::is_reference_v<T>, "Not implemented");
131+
130132
template <typename U> static PyObject *c2py(U &&x) {
131133
PyTypeObject *p = get_type_ptr(typeid(T));
132134
if (p == nullptr) return NULL;

c++/cpp2py/pyref.hpp

+1
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ namespace cpp2py {
131131
return i;
132132
}
133133
};
134+
static_assert(sizeof(pyref) == sizeof(PyObject *), "pyref must contain only a PyObject *");
134135

135136
// FIXME : put static or the other functions inline ?
136137

c++/cpp2py/traits.hpp

+11
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#pragma once
2+
#include <type_traits>
3+
4+
namespace cpp2py {
5+
6+
template <template <typename...> class TMPLT, typename T> struct is_instantiation_of : std::false_type {};
7+
template <template <typename...> class TMPLT, typename... U> struct is_instantiation_of<TMPLT, TMPLT<U...>> : std::true_type {};
8+
template <template <typename...> class gf, typename T>
9+
inline constexpr bool is_instantiation_of_v = is_instantiation_of<gf, std::decay_t<T>>::value;
10+
11+
} // namespace cpp2py

0 commit comments

Comments
 (0)