Skip to content

Commit 64a6011

Browse files
committed
More CI fixes
1 parent 80154dd commit 64a6011

File tree

12 files changed

+151
-82
lines changed

12 files changed

+151
-82
lines changed

.codespell-ignore-lines

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ template <op_id id, op_type ot, typename L = undefined_t, typename R = undefined
22
template <typename ThisT>
33
auto &this_ = static_cast<ThisT &>(*this);
44
if (load_impl<ThisT>(temp, false)) {
5+
return load_impl<ThisT>(src, false);
56
ssize_t nd = 0;
67
auto trivial = broadcast(buffers, nd, shape);
78
auto ndim = (size_t) nd;

include/pybind11/cast.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,10 @@ class type_caster_enum_type {
110110

111111
// NOLINTNEXTLINE(google-explicit-constructor)
112112
operator EnumType &() {
113-
return native_loaded ? native_value
114-
: legacy_ptr ? *legacy_ptr
115-
: throw reference_cast_error();
113+
if (!native_loaded && !legacy_ptr) {
114+
throw reference_cast_error();
115+
}
116+
return native_loaded ? native_value : *legacy_ptr;
116117
}
117118

118119
private:

include/pybind11/detail/common.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,8 @@ inline bool is_uniquely_referenced(PyObject *obj) {
284284
#else // backport for 3.13
285285
inline bool is_uniquely_referenced(PyObject *obj) {
286286
return _Py_IsOwnedByCurrentThread(obj)
287-
&& _Py_atomic_load_uint32_relaxed(&ob->ob_ref_local) == 1
288-
&& _Py_atomic_load_ssize_relaxed(&ob->ob_ref_shared) == 0;
287+
&& _Py_atomic_load_uint32_relaxed(&obj->ob_ref_local) == 1
288+
&& _Py_atomic_load_ssize_relaxed(&obj->ob_ref_shared) == 0;
289289
}
290290
#endif
291291

include/pybind11/detail/foreign.h

Lines changed: 41 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ PYBIND11_NAMESPACE_BEGIN(detail)
2222
PYBIND11_NOINLINE void foreign_exception_translator(std::exception_ptr p) {
2323
auto &interop_internals = get_interop_internals();
2424
for (pymb_framework *fw : interop_internals.exc_frameworks) {
25-
if (fw->translate_exception(&p))
25+
if (fw->translate_exception(&p) != 0) {
2626
return;
27+
}
2728
}
2829
std::rethrow_exception(p);
2930
}
@@ -47,14 +48,14 @@ inline void import_foreign_binding(pymb_binding *binding, const std::type_info *
4748
// Caller must hold the internals lock
4849
auto &interop_internals = get_interop_internals();
4950
interop_internals.imported_any = true;
50-
auto range = interop_internals.bindings.equal_range(*cpptype);
51-
for (auto it = range.first; it != range.second; ++it) {
52-
if (it->second == binding) {
51+
auto &lst = interop_internals.bindings[*cpptype];
52+
for (pymb_binding *existing : lst) {
53+
if (existing == binding) {
5354
return; // already imported
5455
}
5556
}
5657
++interop_internals.bindings_update_count;
57-
interop_internals.bindings.emplace(*cpptype, binding);
58+
lst.append(binding);
5859
}
5960

6061
// Callback functions for other frameworks to operate on our objects
@@ -77,14 +78,14 @@ inline void *interop_cb_from_python(pymb_binding *binding,
7778
= reinterpret_borrow<capsule>(pytype.attr(native_enum_info::attribute_name()));
7879
auto *info = cap.get_pointer<native_enum_info>();
7980
auto value = handle(pyobj).attr("value");
80-
uint64_t ival;
81+
uint64_t ival = 0;
8182
if (info->is_signed && handle(value) < int_(0)) {
8283
ival = (uint64_t) cast<int64_t>(value);
8384
} else {
8485
ival = cast<uint64_t>(value);
8586
}
8687
bytes holder{reinterpret_cast<const char *>(&ival)
87-
+ PYBIND11_BIG_ENDIAN * (8 - info->size_bytes),
88+
+ PYBIND11_BIG_ENDIAN * size_t(8 - info->size_bytes),
8889
info->size_bytes};
8990
keep_referenced(keep_referenced_ctx, holder.ptr());
9091
return PyBytes_AsString(holder.ptr());
@@ -198,7 +199,7 @@ inline PyObject *interop_cb_to_python(pymb_binding *binding,
198199
auto cap
199200
= reinterpret_borrow<capsule>(pytype.attr(native_enum_info::attribute_name()));
200201
auto *info = cap.get_pointer<native_enum_info>();
201-
uint64_t key;
202+
uint64_t key = 0;
202203
switch (info->size_bytes) {
203204
case 1:
204205
key = *(uint8_t *) cobj;
@@ -215,10 +216,11 @@ inline PyObject *interop_cb_to_python(pymb_binding *binding,
215216
default:
216217
return nullptr;
217218
}
218-
if (rvp_ == pymb_rv_policy_take_ownership)
219+
if (rvp_ == pymb_rv_policy_take_ownership) {
219220
::operator delete(cobj);
221+
}
220222
if (info->is_signed) {
221-
int64_t ikey = (int64_t) key;
223+
auto ikey = (int64_t) key;
222224
if (info->size_bytes < 8) {
223225
// sign extend
224226
ikey <<= (64 - (info->size_bytes * 8));
@@ -275,7 +277,7 @@ inline PyObject *interop_cb_to_python(pymb_binding *binding,
275277
srcs.init_instance = init_instance_unregistered;
276278
}
277279
handle ret = type_caster_generic::cast(srcs, rvp, {}, copy_ctor, move_ctor);
278-
feedback->is_new = srcs.is_new;
280+
feedback->is_new = uint8_t(srcs.is_new);
279281
return ret.ptr();
280282
} catch (...) {
281283
translate_exception(std::current_exception());
@@ -309,7 +311,9 @@ inline int interop_cb_keep_alive(PyObject *nurse, void *payload, void (*cb)(void
309311
// Create a shared_ptr whose destruction will perform the action
310312
std::shared_ptr<void> owner(payload, cb_to_use);
311313
// Use the aliasing constructor to make its get() return the right thing
314+
// NB: this constructor accepts an rvalue reference only in C++20
312315
new (std::addressof(v_h.holder<std::shared_ptr<void>>()))
316+
// NOLINTNEXTLINE(performance-move-const-arg)
313317
std::shared_ptr<void>(std::move(owner), v_h.value_ptr());
314318
v_h.set_holder_constructed();
315319
success = true;
@@ -403,13 +407,12 @@ inline int interop_cb_translate_exception(void *eptr) noexcept {
403407
inline void interop_cb_remove_local_binding(pymb_binding *binding) noexcept {
404408
with_internals([&](internals &) {
405409
auto &interop_internals = get_interop_internals();
406-
auto *cpptype = (const std::type_info *) binding->native_type;
407-
auto range = interop_internals.bindings.equal_range(*cpptype);
408-
for (auto it = range.first; it != range.second; ++it) {
409-
if (it->second == binding) {
410-
++interop_internals.bindings_update_count;
410+
const auto *cpptype = (const std::type_info *) binding->native_type;
411+
auto it = interop_internals.bindings.find(*cpptype);
412+
if (it != interop_internals.bindings.end() && it->second.erase(binding)) {
413+
++interop_internals.bindings_update_count;
414+
if (it->second.empty()) {
411415
interop_internals.bindings.erase(it);
412-
return;
413416
}
414417
}
415418
});
@@ -433,12 +436,11 @@ inline void interop_cb_remove_foreign_binding(pymb_binding *binding) noexcept {
433436
with_internals([&](internals &) {
434437
auto &interop_internals = get_interop_internals();
435438
auto remove_from_type = [&](const std::type_info *type) {
436-
auto range = interop_internals.bindings.equal_range(*type);
437-
for (auto it = range.first; it != range.second; ++it) {
438-
if (it->second == binding) {
439-
++interop_internals.bindings_update_count;
439+
auto it = interop_internals.bindings.find(*type);
440+
if (it != interop_internals.bindings.end() && it->second.erase(binding)) {
441+
++interop_internals.bindings_update_count;
442+
if (it->second.empty()) {
440443
interop_internals.bindings.erase(it);
441-
break;
442444
}
443445
}
444446
};
@@ -641,11 +643,10 @@ PYBIND11_NOINLINE void interop_enable_import_all() {
641643
PYBIND11_NOINLINE void
642644
export_for_interop(const std::type_info *cpptype, PyTypeObject *pytype, type_info *ti) {
643645
auto &interop_internals = get_interop_internals();
644-
auto range = interop_internals.bindings.equal_range(*cpptype);
645-
for (auto it = range.first; it != range.second; ++it) {
646-
if (it->second->framework == interop_internals.self.get()
647-
&& it->second->pytype == pytype) {
648-
return; // already exported
646+
auto &lst = interop_internals.bindings[*cpptype];
647+
for (pymb_binding *existing : lst) {
648+
if (existing->framework == interop_internals.self.get() && existing->pytype == pytype) {
649+
return; // already imported
649650
}
650651
}
651652

@@ -657,7 +658,7 @@ export_for_interop(const std::type_info *cpptype, PyTypeObject *pytype, type_inf
657658
binding->context = ti;
658659

659660
++interop_internals.bindings_update_count;
660-
interop_internals.bindings.emplace(*cpptype, binding);
661+
lst.append(binding);
661662
pymb_add_binding(binding, /* tp_finalize_will_remove */ 0);
662663
}
663664

@@ -688,7 +689,7 @@ PYBIND11_NOINLINE void interop_enable_export_all() {
688689
handle(entry.second).attr(native_enum_info::attribute_name()));
689690
auto *info = cap.get_pointer<native_enum_info>();
690691
detail::export_for_interop(info->cpptype, (PyTypeObject *) entry.second, nullptr);
691-
} catch (error_already_set &) {
692+
} catch (error_already_set &) { // NOLINT(bugprone-empty-catch)
692693
// Ignore native enums without a __pybind11_enum__ capsule;
693694
// they might be from an older version of pybind11
694695
}
@@ -710,10 +711,11 @@ PYBIND11_NOINLINE void *try_foreign_bindings(const std::type_info *type,
710711
do {
711712
PYBIND11_LOCK_INTERNALS(internals);
712713
(void) internals; // suppress unused warning on non-ft builds
713-
auto range = interop_internals.bindings.equal_range(*type);
714-
auto it = range.first;
715-
for (; it != range.second; ++it) {
716-
auto *binding = it->second;
714+
auto it = interop_internals.bindings.find(*type);
715+
if (it == interop_internals.bindings.end()) {
716+
return nullptr;
717+
}
718+
for (pymb_binding *binding : it->second) {
717719
if (binding->framework == interop_internals.self.get()
718720
&& (!binding->context
719721
|| !is_local_to_other_module((type_info *) binding->context))) {
@@ -741,13 +743,13 @@ PYBIND11_NOINLINE void *try_foreign_bindings(const std::type_info *type,
741743
// was done within attempt(), or concurrently during attempt()
742744
// while we didn't hold the internals lock
743745
if (interop_internals.bindings_update_count != update_count) {
744-
// Concurrent update occurred; retry
745-
update_count = interop_internals.bindings_update_count;
746+
// Concurrent update occurred; stop iterating
746747
break;
747748
}
748749
}
749-
if (it != range.second) {
750+
if (interop_internals.bindings_update_count != update_count) {
750751
// We broke out early due to a concurrent update. Retry from the top.
752+
update_count = interop_internals.bindings_update_count;
751753
continue;
752754
}
753755
return nullptr;
@@ -775,7 +777,7 @@ inline void import_for_interop(handle pytype) {
775777
auto &interop_internals = detail::get_interop_internals();
776778
interop_internals.initialize_if_needed();
777779
detail::with_internals(
778-
[&](detail::internals &) { detail::import_for_interop(std::move(pytype), cpptype); });
780+
[&](detail::internals &) { detail::import_for_interop(pytype, cpptype); });
779781
}
780782

781783
inline void export_for_interop(handle ty) {
@@ -806,7 +808,8 @@ inline void export_for_interop(handle ty) {
806808
if (ours) {
807809
return;
808810
}
809-
} catch (error_already_set &) {
811+
} catch (error_already_set &) { // NOLINT(bugprone-empty-catch)
812+
// Could be an older native enum without __pybind11_enum__ capsule
810813
}
811814
pybind11_fail("pybind11::export_for_interop: not a "
812815
"pybind11 class or enum bound in this domain");

include/pybind11/detail/internals.h

Lines changed: 65 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,6 @@ struct type_equal_to {
184184

185185
template <typename value_type>
186186
using type_map = std::unordered_map<std::type_index, value_type, type_hash, type_equal_to>;
187-
template <typename value_type>
188-
using type_multimap
189-
= std::unordered_multimap<std::type_index, value_type, type_hash, type_equal_to>;
190187

191188
struct override_hash {
192189
inline size_t operator()(const std::pair<const PyObject *, const char *> &v) const {
@@ -313,10 +310,72 @@ using copy_or_move_ctor = void *(*) (const void *);
313310
// indicating whether any foreign bindings are also known for its C++ type;
314311
// that way we can avoid an extra lookup when conversion to a native type fails.
315312
struct interop_internals {
313+
// A vector optimized for the case of storing one element.
314+
class binding_list {
315+
public:
316+
binding_list() : single(nullptr) {}
317+
binding_list(const binding_list &) = delete;
318+
binding_list(binding_list &&other) noexcept : vec(other.vec) { other.vec = 0; }
319+
~binding_list() {
320+
if (is_vec()) {
321+
delete as_vec();
322+
}
323+
}
324+
325+
bool empty() const { return single == nullptr; }
326+
pymb_binding **begin() {
327+
return empty() ? nullptr : is_vec() ? as_vec()->data() : &single;
328+
}
329+
pymb_binding **end() {
330+
return empty() ? nullptr
331+
: is_vec() ? as_vec()->data() + as_vec()->size()
332+
: &single + 1;
333+
}
334+
void append(pymb_binding *binding) {
335+
if (empty()) {
336+
single = binding;
337+
} else if (!is_vec()) {
338+
vec = uintptr_t(new Vec{single, binding}) | uintptr_t(1);
339+
} else {
340+
as_vec()->push_back(binding);
341+
}
342+
}
343+
bool erase(pymb_binding *binding) {
344+
if (is_vec()) {
345+
Vec *v = as_vec();
346+
for (auto it = v->begin(); it != v->end(); ++it) {
347+
if (*it == binding) {
348+
v->erase(it);
349+
if (v->size() == 1) {
350+
single = (*v)[0];
351+
delete v;
352+
}
353+
return true;
354+
}
355+
}
356+
} else if (single == binding) {
357+
single = nullptr;
358+
return true;
359+
}
360+
return false;
361+
}
362+
363+
private:
364+
// Discriminated pointer: points to pymb_binding if the low bit is clear,
365+
// or to std::vector<pymb_binding> if the low bit is set.
366+
using Vec = std::vector<pymb_binding *>;
367+
union {
368+
pymb_binding *single;
369+
uintptr_t vec;
370+
};
371+
bool is_vec() const { return (vec & uintptr_t(1)) != 0; }
372+
Vec *as_vec() const { return (Vec *) (vec & ~uintptr_t(1)); }
373+
};
374+
316375
// Registered pymetabind bindings for each C++ type, including both our
317376
// own (exported) and other frameworks' (imported).
318377
// Protected by internals::mutex.
319-
type_multimap<pymb_binding *> bindings;
378+
type_map<binding_list> bindings;
320379

321380
// For each C++ type with a native binding, store pointers to its
322381
// copy and move constructors. These would ideally move inside `type_info`
@@ -801,9 +860,8 @@ inline auto with_exception_translators(const F &cb)
801860
}
802861

803862
inline internals_pp_manager<interop_internals> &get_interop_internals_pp_manager() {
804-
static internals_pp_manager<interop_internals> interop_internals_pp_manager(
805-
PYBIND11_INTERNALS_ID "interop", nullptr);
806-
return interop_internals_pp_manager;
863+
return internals_pp_manager<interop_internals>::get_instance(PYBIND11_INTERNALS_ID "interop",
864+
nullptr);
807865
}
808866

809867
inline interop_internals &get_interop_internals() {

include/pybind11/detail/native_enum_data.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ inline void native_enum_data::finalize() {
215215

216216
auto &interop_internals = get_interop_internals();
217217
if (interop_internals.export_all) {
218-
auto info = enum_info.get_pointer<native_enum_info>();
218+
auto *info = enum_info.get_pointer<native_enum_info>();
219219
interop_internals.export_for_interop(
220220
info->cpptype, (PyTypeObject *) py_enum.ptr(), nullptr);
221221
}

include/pybind11/detail/type_caster_base.h

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -266,9 +266,10 @@ PYBIND11_NOINLINE handle get_type_handle(const std::type_info &tp,
266266
auto &interop_internals = detail::get_interop_internals();
267267
if (interop_internals.imported_any) {
268268
handle ret = with_internals([&](internals &) {
269-
auto range = interop_internals.bindings.equal_range(tp);
270-
if (range.first != range.second) {
271-
return handle((PyObject *) range.first->second->pytype);
269+
auto it = interop_internals.bindings.find(tp);
270+
if (it != interop_internals.bindings.end() && !it->second.empty()) {
271+
pymb_binding *binding = *it->second.begin();
272+
return handle((PyObject *) binding->pytype);
272273
}
273274
return handle();
274275
});
@@ -1067,9 +1068,9 @@ class type_caster_generic {
10671068
result_v = try_foreign_bindings(srcs.original.type, attempt, &cap);
10681069
}
10691070

1070-
PyObject *result = (PyObject *) result_v;
1071+
auto *result = (PyObject *) result_v;
10711072
if (result && policy == return_value_policy::reference_internal && srcs.is_new
1072-
&& !srcs.used_foreign->keep_alive(result, parent.ptr(), nullptr)) {
1073+
&& srcs.used_foreign->keep_alive(result, parent.ptr(), nullptr) == 0) {
10731074
keep_alive_impl(result, parent.ptr());
10741075
}
10751076
return result;
@@ -1303,9 +1304,8 @@ class type_caster_generic {
13031304
if (hasattr(pytype, local_key)) {
13041305
return try_load_other_module_local(
13051306
src, reinterpret_borrow<capsule>(getattr(pytype, local_key)));
1306-
} else {
1307-
return foreign_ok && try_load_other_framework(src, convert);
13081307
}
1308+
return foreign_ok && try_load_other_framework(src, convert);
13091309
}
13101310

13111311
// Implementation of `load`; this takes the type of `this` so that it can dispatch the relevant
@@ -1372,7 +1372,7 @@ class type_caster_generic {
13721372
if (convert) {
13731373
for (const auto &converter : typeinfo->implicit_conversions) {
13741374
auto temp = reinterpret_steal<object>(converter(src.ptr(), typeinfo->type));
1375-
if (load_impl<ThisT>(temp, false)) { // codespell:ignore ThisT
1375+
if (load_impl<ThisT>(temp, false)) {
13761376
loader_life_support::add_patient(temp);
13771377
return true;
13781378
}

0 commit comments

Comments
 (0)