Fix test, after internals is free'd it can come back at the same address

So instead, just make sure it was zero'd and don't try to compare the addresses.

Also a little code cleanup

Author: b-pass

include/pybind11/detail/internals.h

@@ -478,7 +478,7 @@ template <typename InternalsType>
 class internals_pp_manager {
 public:
     using on_fetch_function = void(InternalsType *);
-    explicit internals_pp_manager(char const *id, on_fetch_function *on_fetch = nullptr)
+    internals_pp_manager(char const *id, on_fetch_function *on_fetch)
         : holder_id_(id), on_fetch_(on_fetch) {}
 
     /// Get the current pointer-to-pointer, allocating it if it does not already exist.  May
@@ -571,29 +571,35 @@ class internals_pp_manager {
     std::unique_ptr<InternalsType> *internals_singleton_pp_;
 };
 
-inline void on_internals_fetch(internals *internals_ptr) {
-    // If We loaded the internals through `state_dict`, our `error_already_set`
-    // and `builtin_exception` may be different local classes than the ones set up in the
-    // initial exception translator, below, so add another for our local exception classes.
-    //
-    // libstdc++ doesn't require this (types there are identified only by name)
-    // libc++ with CPython doesn't require this (types are explicitly exported)
-    // libc++ with PyPy still need it, awaiting further investigation
-    if (internals_ptr) {
 #if !defined(__GLIBCXX__)
+// If We loaded the internals through `state_dict`, our `error_already_set`
+// and `builtin_exception` may be different local classes than the ones set up in the
+// initial exception translator, below, so add another for our local exception classes.
+//
+// libstdc++ doesn't require this (types there are identified only by name)
+// libc++ with CPython doesn't require this (types are explicitly exported)
+// libc++ with PyPy still need it, awaiting further investigation
+inline void check_internals_local_exception_translator(internals *internals_ptr) {
+    if (internals_ptr) {
         for (auto et : internals_ptr->registered_exception_translators) {
             if (et == &translate_local_exception) {
                 return;
             }
         }
         internals_ptr->registered_exception_translators.push_front(&translate_local_exception);
-#endif
     }
 }
+#endif
 
 inline internals_pp_manager<internals> &get_internals_pp_manager() {
+#if defined(__GLIBCXX__)
+#    define ON_FETCH_FN nullptr
+#else
+#    define ON_FETCH_FN &check_internals_local_exception_translator
+#endif
     static internals_pp_manager<internals> internals_pp_manager(PYBIND11_INTERNALS_ID,
-                                                                &on_internals_fetch);
+                                                                ON_FETCH_FN);
+#undef ON_FETCH_FN
     return internals_pp_manager;
 }
 
@@ -623,7 +629,7 @@ inline internals_pp_manager<local_internals> &get_local_internals_pp_manager() {
         = PYBIND11_MODULE_LOCAL_ID
           + std::to_string(reinterpret_cast<uintptr_t>(&this_module_idstr));
     static internals_pp_manager<local_internals> local_internals_pp_manager(
-        this_module_idstr.c_str());
+        this_module_idstr.c_str(), nullptr);
     return local_internals_pp_manager;
 }
 

tests/test_embed/test_interpreter.cpp

@@ -272,12 +272,11 @@ TEST_CASE("Restart the interpreter") {
     // Verify pre-restart state.
     REQUIRE(py::module_::import("widget_module").attr("add")(1, 2).cast<int>() == 3);
     REQUIRE(has_state_dict_internals_obj());
-    auto internals_addr = get_details_as_uintptr();
     REQUIRE(py::module_::import("external_module").attr("A")(123).attr("value").cast<int>()
             == 123);
 
     // local and foreign module internals should point to the same internals:
-    REQUIRE(internals_addr
+    REQUIRE(get_details_as_uintptr()
             == py::module_::import("external_module").attr("internals_at")().cast<uintptr_t>());
 
     // Restart the interpreter.
@@ -289,11 +288,11 @@ TEST_CASE("Restart the interpreter") {
 
     // Internals are deleted after a restart.
     REQUIRE_FALSE(has_state_dict_internals_obj());
+    REQUIRE(get_details_as_uintptr() == 0);
     pybind11::detail::get_internals();
     REQUIRE(has_state_dict_internals_obj());
-    REQUIRE(internals_addr != get_details_as_uintptr());
-    internals_addr = get_details_as_uintptr();
-    REQUIRE(internals_addr
+    REQUIRE(get_details_as_uintptr() != 0);
+    REQUIRE(get_details_as_uintptr()
             == py::module_::import("external_module").attr("internals_at")().cast<uintptr_t>());
 
     // Make sure that an interpreter with no get_internals() created until finalize still gets the
@@ -313,7 +312,7 @@ TEST_CASE("Restart the interpreter") {
     REQUIRE(ran);
     py::initialize_interpreter();
     REQUIRE_FALSE(has_state_dict_internals_obj());
-    REQUIRE(internals_addr != get_details_as_uintptr());
+    REQUIRE(get_details_as_uintptr() == 0);
 
     // C++ modules can be reloaded.
     auto cpp_module = py::module_::import("widget_module");