Skip to content

Commit 7bf9b82

Browse files
committed
Fix inconsistent pybind11/eigen/tensor.h behavior:
This existing comment in pybind11/eigen/tensor.h ``` // move, take_ownership don't make any sense for a ref/map: ``` is at odds with the `delete src;` three lines up. In real-world client code `take_ownership` will not exist (unless the client code is untested and unused). I.e. the `delete` is essentially only useful to avoid leaks in the pybind11 unit tests. While upgrading to clang-tidy 18, the warning below appeared. Apparently it is produced during LTO, and it appears difficult to suppress. Regardless, the best way to resolve this is to remove the `delete` and to simply make the test objects `static` in the unit test code. ________ Debian clang version 18.1.8 (++20240718080534+3b5b5c1ec4a3-1~exp1~20240718200641.143) Target: x86_64-pc-linux-gnu ________ ``` lto-wrapper: warning: using serial compilation of 3 LTRANS jobs lto-wrapper: note: see the ‘-flto’ option documentation for more information In function ‘cast_impl’, inlined from ‘cast’ at /mounted_pybind11/include/pybind11/eigen/tensor.h:414:25, inlined from ‘operator()’ at /mounted_pybind11/include/pybind11/eigen/../pybind11.h:296:40, inlined from ‘_FUN’ at /mounted_pybind11/include/pybind11/eigen/../pybind11.h:267:21: /mounted_pybind11/include/pybind11/eigen/tensor.h:475:17: warning: ‘operator delete’ called on unallocated object ‘<anonymous>’ [-Wfree-nonheap-object] 475 | delete src; | ^ /mounted_pybind11/include/pybind11/eigen/../pybind11.h: In function ‘_FUN’: /mounted_pybind11/include/pybind11/eigen/../pybind11.h:297:75: note: declared here 297 | std::move(args_converter).template call<Return, Guard>(cap->f), | ^ ```
1 parent 3e5fb45 commit 7bf9b82

File tree

2 files changed

+11
-8
lines changed

2 files changed

+11
-8
lines changed

include/pybind11/eigen/tensor.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -469,9 +469,6 @@ struct type_caster<Eigen::TensorMap<Type, Options>,
469469
parent_object = reinterpret_borrow<object>(parent);
470470
break;
471471

472-
case return_value_policy::take_ownership:
473-
delete src;
474-
// fallthrough
475472
default:
476473
// move, take_ownership don't make any sense for a ref/map:
477474
pybind11_fail("Invalid return_value_policy for Eigen Map type, must be either "

tests/test_eigen_tensor.inl

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,33 +147,39 @@ void init_tensor_module(pybind11::module &m) {
147147

148148
m.def(
149149
"take_fixed_tensor",
150-
151150
[]() {
152151
Eigen::aligned_allocator<
153152
Eigen::TensorFixedSize<double, Eigen::Sizes<3, 5, 2>, Options>>
154153
allocator;
155-
return new (allocator.allocate(1))
154+
static auto *obj = new (allocator.allocate(1))
156155
Eigen::TensorFixedSize<double, Eigen::Sizes<3, 5, 2>, Options>(
157156
get_fixed_tensor<Options>());
157+
return obj; // take_ownership will fail.
158158
},
159159
py::return_value_policy::take_ownership);
160160

161161
m.def(
162162
"take_tensor",
163-
[]() { return new Eigen::Tensor<double, 3, Options>(get_tensor<Options>()); },
163+
[]() {
164+
static auto *obj = new Eigen::Tensor<double, 3, Options>(get_tensor<Options>());
165+
return obj; // take_ownership will fail.
166+
},
164167
py::return_value_policy::take_ownership);
165168

166169
m.def(
167170
"take_const_tensor",
168171
[]() -> const Eigen::Tensor<double, 3, Options> * {
169-
return new Eigen::Tensor<double, 3, Options>(get_tensor<Options>());
172+
static auto *obj = new Eigen::Tensor<double, 3, Options>(get_tensor<Options>());
173+
return obj; // take_ownership will fail.
170174
},
171175
py::return_value_policy::take_ownership);
172176

173177
m.def(
174178
"take_view_tensor",
175179
[]() -> const Eigen::TensorMap<Eigen::Tensor<double, 3, Options>> * {
176-
return new Eigen::TensorMap<Eigen::Tensor<double, 3, Options>>(get_tensor<Options>());
180+
static auto *obj
181+
= new Eigen::TensorMap<Eigen::Tensor<double, 3, Options>>(get_tensor<Options>());
182+
return obj; // take_ownership will fail.
177183
},
178184
py::return_value_policy::take_ownership);
179185

0 commit comments

Comments
 (0)