Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use std::hash<std::type_index>, std::equal_to<std::type_index> everywhere **except when libc++ is in use** #4319

Merged
merged 29 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a06949a
Try using `std::hash<std::type_index>`, `std::equal_to<std::type_inde…
Nov 6, 2022
9d70eba
Revert "Try using `std::hash<std::type_index>`, `std::equal_to<std::t…
Nov 7, 2022
9141519
Use "our own name-based hash and equality functions" for `std::type_i…
Nov 7, 2022
ba747dc
Patch in PR #4313: Minimal reproducer for clash when binding types de…
Nov 7, 2022
1912670
test_unnamed_namespace_b xfail for clang
Nov 7, 2022
d8b7118
`PYBIND11_INTERNALS_VERSION 5`
Nov 7, 2022
780d3b2
Add a note to docs/classes.rst
Nov 7, 2022
3373b47
For compatibility with Google-internal testing, test_unnamed_namespac…
Nov 7, 2022
8f0c28e
Trying "__GLIBCXX__ or Windows", based on observations from Google-in…
Nov 7, 2022
883cadf
Try _LIBCPP_VERSION
Nov 7, 2022
72a8895
Account for libc++ behavior in tests and documentation.
Nov 7, 2022
6f1ddb2
Adjust expectations for Windows Clang (and make code less redundant).
Nov 10, 2022
a1175ed
Add WindowsClang to ci.yml
Nov 10, 2022
5631d94
Add clang-latest to name that appears in the GitHub Actions web view.
Nov 10, 2022
81a4e7b
Tweak the note in classes.rst again.
Nov 10, 2022
7075e00
Add `pip install --upgrade pip`, Show env, cosmetic changes
Nov 10, 2022
b84a163
Add macos_brew_install_llvm to ci.yml
Nov 10, 2022
4f06c06
`test_cross_module_exception_translator` xfail 'Homebrew Clang'
Nov 10, 2022
2180d04
Revert back to base version of .github/workflows/ci.yml (the ci.yml c…
Mar 20, 2023
d2d2c0a
Merge branch 'master' into internals_std_type_index_modernization
Mar 20, 2023
b882060
Fixes for ruff
Mar 20, 2023
66e0fac
Merge branch 'master' into internals_std_type_index_modernization
Mar 28, 2023
f1c3055
Make updated condition in internals.h dependent on ABI version.
Mar 28, 2023
4842b9f
Merge branch 'master' into internals_std_type_index_modernization
Mar 30, 2023
0ff73a9
Remove PYBIND11_TEST_OVERRIDE when testing with PYBIND11_INTERNALS_VE…
Mar 30, 2023
9f60e9a
Merge branch 'master' into internals_std_type_index_modernization
Apr 21, 2023
d0276c0
Selectively exercise cmake `-DPYBIND11_TEST_OVERRIDE`: ubuntu, macos,…
Apr 21, 2023
4f4fd04
Merge branch 'master' into internals_std_type_index_modernization
Apr 25, 2023
1b4a508
Update skipif for Python 3.12a7 (the WIP needs to be handled in a sep…
Apr 25, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/workflows/upstream.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ jobs:
-DDOWNLOAD_EIGEN=ON
-DCMAKE_CXX_STANDARD=17
-DPYBIND11_INTERNALS_VERSION=10000000
"-DPYBIND11_TEST_OVERRIDE=test_call_policies.cpp;test_gil_scoped.cpp;test_thread.cpp"

- name: Build (unstable ABI)
run: cmake --build build17max -j 2
Expand Down
10 changes: 10 additions & 0 deletions docs/classes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ interactive Python session demonstrating this example is shown below:
Static member functions can be bound in the same way using
:func:`class_::def_static`.

.. note::

Binding C++ types in unnamed namespaces (also known as anonymous namespaces)
works reliably on many platforms, but not all. The `XFAIL_CONDITION` in
tests/test_unnamed_namespace_a.py encodes the currently known conditions.
For background see `#4319 <https://github.com/pybind/pybind11/pull/4319>`_.
If portability is a concern, it is therefore not recommended to bind C++
types in unnamed namespaces. It will be safest to manually pick unique
namespace names.

Keyword and default arguments
=============================
It is possible to specify keyword and default arguments using the syntax
Expand Down
3 changes: 2 additions & 1 deletion include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,8 @@ inline void tls_replace_value(PYBIND11_TLS_KEY_REF key, void *value) {
// libstdc++, this doesn't happen: equality and the type_index hash are based on the type name,
// which works. If not under a known-good stl, provide our own name-based hash and equality
// functions that use the type name.
#if defined(__GLIBCXX__)
#if (PYBIND11_INTERNALS_VERSION <= 4 && defined(__GLIBCXX__)) \
|| (PYBIND11_INTERNALS_VERSION >= 5 && !defined(_LIBCPP_VERSION))
inline bool same_type(const std::type_info &lhs, const std::type_info &rhs) { return lhs == rhs; }
using type_hash = std::hash<std::type_index>;
using type_equal_to = std::equal_to<std::type_index>;
Expand Down
2 changes: 2 additions & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ set(PYBIND11_TEST_FILES
test_tagbased_polymorphic
test_thread
test_union
test_unnamed_namespace_a
test_unnamed_namespace_b
test_vector_unique_ptr_member
test_virtual_functions)

Expand Down
2 changes: 1 addition & 1 deletion tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ def test_error_already_set_what_with_happy_exceptions(

@pytest.mark.skipif(
# Intentionally very specific:
"sys.version_info == (3, 12, 0, 'alpha', 6)",
"sys.version_info == (3, 12, 0, 'alpha', 7)",
reason="WIP: https://github.com/python/cpython/issues/102594",
)
@pytest.mark.skipif("env.PYPY", reason="PyErr_NormalizeException Segmentation fault")
Expand Down
38 changes: 38 additions & 0 deletions tests/test_unnamed_namespace_a.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#include "pybind11_tests.h"

namespace {
struct any_struct {};
} // namespace

TEST_SUBMODULE(unnamed_namespace_a, m) {
if (py::detail::get_type_info(typeid(any_struct)) == nullptr) {
py::class_<any_struct>(m, "unnamed_namespace_a_any_struct");
} else {
m.attr("unnamed_namespace_a_any_struct") = py::none();
}
m.attr("PYBIND11_INTERNALS_VERSION") = PYBIND11_INTERNALS_VERSION;
m.attr("defined_WIN32_or__WIN32") =
#if defined(WIN32) || defined(_WIN32)
true;
#else
false;
#endif
m.attr("defined___clang__") =
#if defined(__clang__)
true;
#else
false;
#endif
m.attr("defined__LIBCPP_VERSION") =
#if defined(_LIBCPP_VERSION)
true;
#else
false;
#endif
m.attr("defined___GLIBCXX__") =
#if defined(__GLIBCXX__)
true;
#else
false;
#endif
}
34 changes: 34 additions & 0 deletions tests/test_unnamed_namespace_a.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import pytest

from pybind11_tests import unnamed_namespace_a as m
from pybind11_tests import unnamed_namespace_b as mb

XFAIL_CONDITION = (
"(m.PYBIND11_INTERNALS_VERSION <= 4 and (m.defined___clang__ or not m.defined___GLIBCXX__))"
" or "
"(m.PYBIND11_INTERNALS_VERSION >= 5 and not m.defined_WIN32_or__WIN32"
" and "
"(m.defined___clang__ or m.defined__LIBCPP_VERSION))"
)
XFAIL_REASON = "Known issues: https://github.com/pybind/pybind11/pull/4319"


@pytest.mark.xfail(XFAIL_CONDITION, reason=XFAIL_REASON, strict=False)
@pytest.mark.parametrize(
"any_struct", [m.unnamed_namespace_a_any_struct, mb.unnamed_namespace_b_any_struct]
)
def test_have_class_any_struct(any_struct):
assert any_struct is not None


def test_have_at_least_one_class_any_struct():
assert (
m.unnamed_namespace_a_any_struct is not None
or mb.unnamed_namespace_b_any_struct is not None
)


@pytest.mark.xfail(XFAIL_CONDITION, reason=XFAIL_REASON, strict=True)
def test_have_both_class_any_struct():
assert m.unnamed_namespace_a_any_struct is not None
assert mb.unnamed_namespace_b_any_struct is not None
13 changes: 13 additions & 0 deletions tests/test_unnamed_namespace_b.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#include "pybind11_tests.h"

namespace {
struct any_struct {};
} // namespace

TEST_SUBMODULE(unnamed_namespace_b, m) {
if (py::detail::get_type_info(typeid(any_struct)) == nullptr) {
py::class_<any_struct>(m, "unnamed_namespace_b_any_struct");
} else {
m.attr("unnamed_namespace_b_any_struct") = py::none();
}
}
5 changes: 5 additions & 0 deletions tests/test_unnamed_namespace_b.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from pybind11_tests import unnamed_namespace_b as m


def test_have_attr_any_struct():
assert hasattr(m, "unnamed_namespace_b_any_struct")