Skip to content

Commit 8737bea

Browse files
rwgkrwgk
committed
Split out get_native_enum_type_map(), leaving struct internals untouched (compared to master).
Retested with all sanitizers.
1 parent dd0147a commit 8737bea

File tree

6 files changed

+93
-32
lines changed

6 files changed

+93
-32
lines changed

‎include/pybind11/cast.h

+4-4
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ class type_caster_enum_type {
5858

5959
template <typename SrcType>
6060
static handle cast(SrcType &&src, return_value_policy, handle parent) {
61-
auto const &natives = get_internals().native_enum_types;
61+
auto const &natives = get_native_enum_type_map();
6262
auto found = natives.find(std::type_index(typeid(EnumType)));
6363
if (found != natives.end()) {
6464
return handle(found->second)(static_cast<Underlying>(src)).release();
@@ -71,7 +71,7 @@ class type_caster_enum_type {
7171
}
7272

7373
bool load(handle src, bool convert) {
74-
auto const &natives = get_internals().native_enum_types;
74+
auto const &natives = get_native_enum_type_map();
7575
auto found = natives.find(std::type_index(typeid(EnumType)));
7676
if (found != natives.end()) {
7777
if (!isinstance(src, found->second)) {
@@ -125,7 +125,7 @@ class type_caster<EnumType,
125125

126126
template <typename T, detail::enable_if_t<std::is_enum<T>::value, int> = 0>
127127
bool isinstance_native_enum_impl(handle obj, const std::type_info &tp) {
128-
auto const &natives = get_internals().native_enum_types;
128+
auto const &natives = get_native_enum_type_map();
129129
auto found = natives.find(tp);
130130
if (found == natives.end()) {
131131
return false;
@@ -1138,7 +1138,7 @@ T cast(const handle &handle) {
11381138
"Unable to cast type to reference: value is local to type caster");
11391139
#ifndef NDEBUG
11401140
if (is_enum_cast) {
1141-
auto const &natives = get_internals().native_enum_types;
1141+
auto const &natives = get_native_enum_type_map();
11421142
auto found = natives.find(std::type_index(typeid(intrinsic_t<T>)));
11431143
if (found != natives.end()) {
11441144
pybind11_fail("Unable to cast native enum type to reference");

‎include/pybind11/detail/internals.h

+75-25
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
/// further ABI-incompatible changes may be made before the ABI is officially
3535
/// changed to the new version.
3636
#ifndef PYBIND11_INTERNALS_VERSION
37-
# define PYBIND11_INTERNALS_VERSION 5
37+
# define PYBIND11_INTERNALS_VERSION 4
3838
#endif
3939

4040
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
@@ -209,9 +209,6 @@ struct internals {
209209
PYBIND11_TLS_FREE(tstate);
210210
}
211211
#endif
212-
#if PYBIND11_INTERNALS_VERSION > 4
213-
type_map<PyObject *> native_enum_types;
214-
#endif
215212
};
216213

217214
/// Additional type information which does not fit into the PyTypeObject.
@@ -300,15 +297,16 @@ struct type_info {
300297
# endif
301298
#endif
302299

300+
#define PYBIND11_ABI_ID \
301+
PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB PYBIND11_BUILD_ABI \
302+
PYBIND11_BUILD_TYPE
303+
303304
#define PYBIND11_INTERNALS_ID \
304-
"__pybind11_internals_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) \
305-
PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB PYBIND11_BUILD_ABI \
306-
PYBIND11_BUILD_TYPE "__"
305+
"__pybind11_internals_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) PYBIND11_ABI_ID "__"
307306

308307
#define PYBIND11_MODULE_LOCAL_ID \
309-
"__pybind11_module_local_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) \
310-
PYBIND11_INTERNALS_KIND PYBIND11_COMPILER_TYPE PYBIND11_STDLIB PYBIND11_BUILD_ABI \
311-
PYBIND11_BUILD_TYPE "__"
308+
"__pybind11_module_local_v" PYBIND11_TOSTRING(PYBIND11_INTERNALS_VERSION) PYBIND11_ABI_ID "_" \
309+
"_"
312310

313311
/// Each module locally stores a pointer to the `internals` data. The data
314312
/// itself is shared among modules with the same `PYBIND11_INTERNALS_ID`.
@@ -445,28 +443,29 @@ inline object get_python_state_dict() {
445443
return state_dict;
446444
}
447445

446+
#if defined(WITH_THREAD)
447+
# if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
448+
using gil_scoped_acquire_simple = gil_scoped_acquire;
449+
# else
450+
// Cannot use py::gil_scoped_acquire here since that constructor calls get_internals.
451+
struct gil_scoped_acquire_simple {
452+
gil_scoped_acquire_simple() : state(PyGILState_Ensure()) {}
453+
gil_scoped_acquire_simple(const gil_scoped_acquire_simple &) = delete;
454+
gil_scoped_acquire_simple &operator=(const gil_scoped_acquire_simple &) = delete;
455+
~gil_scoped_acquire_simple() { PyGILState_Release(state); }
456+
const PyGILState_STATE state;
457+
};
458+
# endif
459+
#endif
460+
448461
/// Return a reference to the current `internals` data
449462
PYBIND11_NOINLINE internals &get_internals() {
450463
internals **&internals_pp = get_internals_pp();
451464
if (internals_pp && *internals_pp) {
452465
return **internals_pp;
453466
}
454467

455-
#if defined(WITH_THREAD)
456-
# if defined(PYBIND11_SIMPLE_GIL_MANAGEMENT)
457-
gil_scoped_acquire gil;
458-
# else
459-
// Ensure that the GIL is held since we will need to make Python calls.
460-
// Cannot use py::gil_scoped_acquire here since that constructor calls get_internals.
461-
struct gil_scoped_acquire_local {
462-
gil_scoped_acquire_local() : state(PyGILState_Ensure()) {}
463-
gil_scoped_acquire_local(const gil_scoped_acquire_local &) = delete;
464-
gil_scoped_acquire_local &operator=(const gil_scoped_acquire_local &) = delete;
465-
~gil_scoped_acquire_local() { PyGILState_Release(state); }
466-
const PyGILState_STATE state;
467-
} gil;
468-
# endif
469-
#endif
468+
gil_scoped_acquire_simple gil;
470469
error_scope err_scope;
471470

472471
constexpr const char *id_cstr = PYBIND11_INTERNALS_ID;
@@ -646,4 +645,55 @@ T &get_or_create_shared_data(const std::string &name) {
646645
return *ptr;
647646
}
648647

648+
PYBIND11_NAMESPACE_BEGIN(detail)
649+
650+
#define PYBIND11_NATIVE_ENUM_TYPE_MAP_ABI_ID \
651+
"__pybind11_native_enum_type_map_v1" PYBIND11_ABI_ID "__"
652+
653+
using native_enum_type_map = type_map<PyObject *>;
654+
655+
inline native_enum_type_map **&get_native_enum_types_pp() {
656+
static native_enum_type_map **native_enum_types_pp = nullptr;
657+
return native_enum_types_pp;
658+
}
659+
660+
PYBIND11_NOINLINE native_enum_type_map &get_native_enum_type_map() {
661+
native_enum_type_map **&native_enum_type_map_pp = get_native_enum_types_pp();
662+
if (native_enum_type_map_pp && *native_enum_type_map_pp) {
663+
return **native_enum_type_map_pp;
664+
}
665+
666+
gil_scoped_acquire_simple gil;
667+
error_scope err_scope;
668+
669+
constexpr const char *id_cstr = PYBIND11_NATIVE_ENUM_TYPE_MAP_ABI_ID;
670+
str id(id_cstr);
671+
672+
dict state_dict = get_python_state_dict();
673+
674+
if (state_dict.contains(id_cstr)) {
675+
void *raw_ptr = PyCapsule_GetPointer(state_dict[id].ptr(), id_cstr);
676+
if (raw_ptr == nullptr) {
677+
raise_from(PyExc_SystemError,
678+
"pybind11::detail::get_native_enum_type_map(): Retrieve "
679+
"native_enum_type_map** from capsule FAILED");
680+
}
681+
native_enum_type_map_pp = static_cast<native_enum_type_map **>(raw_ptr);
682+
}
683+
684+
if (native_enum_type_map_pp && *native_enum_type_map_pp) {
685+
return **native_enum_type_map_pp;
686+
}
687+
688+
if (!native_enum_type_map_pp) {
689+
native_enum_type_map_pp = new native_enum_type_map *();
690+
}
691+
auto *&native_enum_type_map_ptr = *native_enum_type_map_pp;
692+
native_enum_type_map_ptr = new native_enum_type_map();
693+
state_dict[id] = capsule(native_enum_type_map_pp, id_cstr);
694+
return **native_enum_type_map_pp;
695+
}
696+
697+
PYBIND11_NAMESPACE_END(detail)
698+
649699
PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)

‎include/pybind11/native_enum.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class native_enum : public detail::native_enum_data {
3232
"pybind11::native_enum<...>(\"" + enum_name_encoded
3333
+ "\") is already registered as a `pybind11::enum_` or `pybind11::class_`!");
3434
}
35-
if (detail::get_internals().native_enum_types.count(enum_type_index)) {
35+
if (detail::get_native_enum_type_map().count(enum_type_index)) {
3636
pybind11_fail("pybind11::native_enum<...>(\"" + enum_name_encoded
3737
+ "\") is already registered!");
3838
}

‎include/pybind11/pybind11.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -1296,7 +1296,7 @@ class module_ : public object {
12961296
py_enum[doc[int_(0)]].attr("__doc__") = doc[int_(1)];
12971297
}
12981298
// Intentionally leak Python reference.
1299-
detail::get_internals().native_enum_types[data.enum_type_index] = py_enum.release().ptr();
1299+
detail::get_native_enum_type_map()[data.enum_type_index] = py_enum.release().ptr();
13001300
return *this;
13011301
}
13021302
};
@@ -2204,7 +2204,7 @@ class enum_ : public class_<Type> {
22042204
enum_(const handle &scope, const char *name, const Extra &...extra)
22052205
: class_<Type>(scope, name, extra...), m_base(*this, scope) {
22062206
{
2207-
auto const &natives = detail::get_internals().native_enum_types;
2207+
auto const &natives = detail::get_native_enum_type_map();
22082208
auto found = natives.find(std::type_index(typeid(Type)));
22092209
if (found != natives.end()) {
22102210
pybind11_fail("pybind11::enum_ \"" + std::string(name)

‎tests/test_native_enum.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE)
6767
TEST_SUBMODULE(native_enum, m) {
6868
using namespace test_native_enum;
6969

70+
m.attr("PYBIND11_NATIVE_ENUM_TYPE_MAP_ABI_ID") = PYBIND11_NATIVE_ENUM_TYPE_MAP_ABI_ID;
71+
7072
m += py::native_enum<smallenum>("smallenum")
7173
.value("a", smallenum::a)
7274
.value("b", smallenum::b)

‎tests/test_native_enum.py

+9
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
11
import enum
2+
import re
23

34
import pytest
45

56
from pybind11_tests import native_enum as m
67

8+
9+
def test_abi_id():
10+
assert re.match(
11+
"__pybind11_native_enum_type_map_v1_.*__$",
12+
m.PYBIND11_NATIVE_ENUM_TYPE_MAP_ABI_ID,
13+
)
14+
15+
716
SMALLENUM_MEMBERS = (
817
("a", 0),
918
("b", 1),

0 commit comments

Comments
 (0)