[master] Debugging test_cross_module_exception_translator issue

[rwgk]

Description

This PR was used to debug this failure (as encountered originally under PR #4050).

https://github.com/pybind/pybind11/runs/7245374556?check_suite_focus=true

____________________ test_cross_module_exception_translator ____________________
    @pytest.mark.xfail(
        "env.PYPY and env.MACOS",
        raises=RuntimeError,
        reason="Expected failure with PyPy and libc++ (Issue #2847 & PR #2[99](https://github.com/pybind/pybind11/runs/7245374556?check_suite_focus=true#step:12:100)9)",
    )
    def test_cross_module_exception_translator():
        with pytest.raises(KeyError):
            # translator registered in cross_module_tests
>           m.throw_should_be_translated_to_key_error()
E           RuntimeError
test_exceptions.py:82: RuntimeError

After a lot of trial-and-error, it turns out that renaming test_namespace_visibility.py to test_exc_namespace_visibility.py, so that it is imported by pytest before test_exceptions.py, "fixes" the failure.

This smells like a bug in register_exception_translator, and a really evil one if it is true. But I'll run with the workaround for now and will leave further debugging for another day.

The idea for the workaround was based on the observation that test_class_sh_module_local.py has been working without a problem for many months, and the guess (still unproven) that the exception translator registry is somehow being reset when a new PYBIND11_MODULE is imported.

Suggested changelog entry:

[rwgk]

The one failing job is a notorious flake (test_iostream).

Everything else passes after renaming test_namespace_visibility.py to test_exc_namespace_visibility.py, so that it is imported by pytest before test_exceptions.py.

This smells like a bug in register_exception_translator, and a really evil one if it is true.

[rwgk]

JIC this is useful for debugging later, the jobs that failed with https://github.com/pybind/pybind11/actions/runs/2638676503 (logs_21304.zip) were:

1______3.6_____MSVC_2019_____x86.txt
1______3.8_____MSVC_2019__Debug______x86_-DCMAKE_CXX_STANDARD=17.txt
1______3.9_____MSVC_2022_C++20_____x64.txt
2______3.7_____MSVC_2019_____x86.txt
2______3.9_____MSVC_2019__Debug______x86_-DCMAKE_CXX_STANDARD=20.txt
3______3.8_____MSVC_2019_____x86_-DCMAKE_CXX_STANDARD=17.txt
4______3.9_____MSVC_2019_____x86_-DCMAKE_CXX_STANDARD=20.txt
8______3.6_____windows-2022_____x64.txt
9______3.9_____windows-2022_____x64.txt
10______3.10_____windows-2022_____x64.txt
11______3.11-dev_____windows-2022_____x64.txt
12______pypy-3.7_____windows-2022_____x64.txt
13______pypy-3.8_____windows-2022_____x64.txt
14______pypy-3.9_____windows-2022_____x64.txt
15______3.6_____macos-latest_____x64.txt
16______3.9_____macos-latest_____x64.txt
17______3.10_____macos-latest_____x64.txt
18______3.11-dev_____macos-latest_____x64.txt
22______3.6_____windows-2019_____x64_-DPYBIND11_FINDPYTHON=ON.txt
23______3.9_____windows-2019_____x64.txt

[rwgk]

Another data point that may or may not be useful when debugging the import order issue:

Under PR #4069, with -fvisibility=hidden removed, and all "... declared with greater visibility than ..." warnings across all platforms resolved in the way as shown in the diff below, the list of jobs that fail is still exactly like shown under #4054 (comment) above (data from https://github.com/pybind/pybind11/runs/7368169837, logs_21599.zip).

diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h
index aea1987b..39531114 100644
--- a/include/pybind11/detail/common.h
+++ b/include/pybind11/detail/common.h
@@ -25,8 +25,10 @@
 // on the main `pybind11` namespace.
 #if !defined(PYBIND11_NAMESPACE)
 #    ifdef __GNUG__
-#        define PYBIND11_NAMESPACE pybind11 __attribute__((visibility("hidden")))
+#        define PYBIND11_NS_VISIBILITY(...) __VA_ARGS__ __attribute__((visibility("hidden")))
+#        define PYBIND11_NAMESPACE PYBIND11_NS_VISIBILITY(pybind11)
 #    else
+#        define PYBIND11_NS_VISIBILITY(...) __VA_ARGS__
 #        define PYBIND11_NAMESPACE pybind11
 #    endif
 #endif
diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp
index 3ec999d1..25f48f78 100644
--- a/tests/test_exceptions.cpp
+++ b/tests/test_exceptions.cpp
@@ -15,6 +15,8 @@
 #include <stdexcept>
 #include <utility>

+PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests))
+
 // A type that should be raised as an exception in Python
 class MyException : public std::exception {
 public:
@@ -110,7 +112,11 @@ std::string error_already_set_what(const py::object &exc_type, const py::object
     return py::error_already_set().what();
 }

+PYBIND11_NAMESPACE_END(pybind11_tests)
+
 TEST_SUBMODULE(exceptions, m) {
+    using namespace pybind11_tests;
+
     m.def("throw_std_exception",
           []() { throw std::runtime_error("This exception was intentionally thrown."); });

diff --git a/tests/test_exceptions.h b/tests/test_exceptions.h
index 03684b89..adad7c6c 100644
--- a/tests/test_exceptions.h
+++ b/tests/test_exceptions.h
@@ -5,9 +5,13 @@

 // shared exceptions for cross_module_tests

+PYBIND11_NAMESPACE_BEGIN(PYBIND11_NS_VISIBILITY(pybind11_tests))
+
 class PYBIND11_EXPORT_EXCEPTION shared_exception : public pybind11::builtin_exception {
 public:
     using builtin_exception::builtin_exception;
     explicit shared_exception() : shared_exception("") {}
     void set_error() const override { PyErr_SetString(PyExc_RuntimeError, what()); }
 };
+
+PYBIND11_NAMESPACE_END(pybind11_tests)

[rwgk]

Re-testing with smart_holder @ commit 6df8693 (which includes #4098):

https://github.com/rwgk/pybind11/tree/sh_rename_test_exc_namespace_visibility

The test_cross_module_exception_translator failure still exists:

platform darwin -- Python 3.8.2, pytest-7.1.2, pluggy-1.0.0
C++ Info: Apple LLVM 12.0.0 (clang-1200.0.31.1) C++17 __pybind11_internals_v4_clang_libcpp_cxxabi1002_sh_def__
...
================================================================= FAILURES =================================================================
__________________________________________________ test_cross_module_exception_translator __________________________________________________

    @pytest.mark.xfail(
        "env.PYPY and env.MACOS",
        raises=RuntimeError,
        reason="Expected failure with PyPy and libc++ (Issue #2847 & PR #2999)",
    )
    def test_cross_module_exception_translator():
        with pytest.raises(KeyError):
            # translator registered in cross_module_tests
>           m.throw_should_be_translated_to_key_error()
E           RuntimeError


../../pybind11/tests/test_exceptions.py:82: RuntimeError