Skip to content

Commit 9ea39dc

Browse files
Force the builtin module key to be the correct type. (#2814)
* Force the builtin module key to be the correct type. Previously it was always going to be a std::string which converted into unicode. Python 2 appears to want module keys to be normal str types, so this was breaking code that expected plain string types in the builtins.keys() data structure * Add a simple unit test to ensure all built-in keys are str * Update the unit test so it will also run on pypy * Run pre-commit. Co-authored-by: Jesse Clemens <[email protected]>
1 parent 08bca37 commit 9ea39dc

File tree

2 files changed

+14
-1
lines changed

2 files changed

+14
-1
lines changed

include/pybind11/detail/internals.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ PYBIND11_NOINLINE inline internals &get_internals() {
266266
const PyGILState_STATE state;
267267
} gil;
268268

269-
constexpr auto *id = PYBIND11_INTERNALS_ID;
269+
PYBIND11_STR_TYPE id(PYBIND11_INTERNALS_ID);
270270
auto builtins = handle(PyEval_GetBuiltins());
271271
if (builtins.contains(id) && isinstance<capsule>(builtins[id])) {
272272
internals_pp = static_cast<internals **>(capsule(builtins[id]));

tests/test_modules.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,16 @@ def test_duplicate_registration():
7575
"""Registering two things with the same name"""
7676

7777
assert m.duplicate_registration() == []
78+
79+
80+
def test_builtin_key_type():
81+
"""Test that all the keys in the builtin modules have type str.
82+
83+
Previous versions of pybind11 would add a unicode key in python 2.
84+
"""
85+
if hasattr(__builtins__, "keys"):
86+
keys = __builtins__.keys()
87+
else: # this is to make pypy happy since builtins is different there.
88+
keys = __builtins__.__dict__.keys()
89+
90+
assert {type(k) for k in keys} == {str}

0 commit comments

Comments
 (0)