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

[Experiment] std::type_index & un/named namespace #4316

Closed
wants to merge 3 commits into from

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Nov 6, 2022

Description

Related to PRs #4313, #4315, #4319

EDIT 2024-01-24: Related LLVM issue: llvm/llvm-project#79367

Suggested changelog entry:

@rwgk
Copy link
Collaborator Author

rwgk commented Nov 6, 2022

The CI results for the current commit are here (everything succeeded):

https://github.com/pybind/pybind11/actions/runs/3405111546/jobs/5662840637

The name of the "Download log archive" file: logs_24760.zip

Results of inspecting the logs are below.

Summary:

It looks like the !__GLIBCXX__ code path in internals.h is no longer needed! In fact, it's only making things worse. The only unreliable platform is Linux & clang, where that code path is not used and wouldn't help.

$ grep 'std::type_index-EQ-BAD' *.txt | sed 's/^[1-9][0-9]*//' | cutniq | sort
______3_____Clang_10_____C++17_____x64.txt
______3_____Clang_10_____C++20_____x64.txt
______3_____Clang_11_____C++20_____x64.txt
______3_____Clang_12_____C++20_____x64.txt
______3_____Clang_13_____C++20_____x64.txt
______3_____Clang_14_____C++20_____x64.txt
______3_____Clang_3.6_____C++11_____x64.txt
______3_____Clang_3.7_____C++11_____x64.txt
______3_____Clang_3.9_____C++11_____x64.txt
______3_____Clang_5_____C++14_____x64.txt
______3_____Clang_7_____C++11_____x64.txt
______3_____Clang_9_____C++11_____x64.txt
______3_____Clang_dev_____C++11_____x64.txt
$ grep 'std::type_index-NE-GOOD' *.txt | sed 's/^[1-9][0-9]*//' | cutniq | sort
______3.10_____CUDA_11.7_____Ubuntu_22.04.txt
______3.10_____macos-latest_____x64.txt
______3.10_____ubuntu-latest_____x64.txt
______3.10_____windows-2022_____x64.txt
______3.11__deadsnakes______x64.txt
______3.11_____macos-latest_____x64.txt
______3.11_____ubuntu-latest_____x64.txt
______3.11_____windows-2022_____x64.txt
______3.6_____macos-latest_____x64.txt
______3.6_____MSVC_2019_____x86.txt
______3.6_____ubuntu-latest_____x64_-DPYBIND11_FINDPYTHON=ON_-DCM.txt
______3.6_____windows-2019_____x64_-DPYBIND11_FINDPYTHON=ON.txt
______3.6_____windows-2022_____x64.txt
______3.7_____Debian_____x86______Install.txt
______3.7_____MSVC_2019_____x86_-DCMAKE_CXX_STANDARD=14.txt
______3.8_____MSVC_2019__Debug______x86_-DCMAKE_CXX_STANDARD=17.txt
______3.8_____MSVC_2019_____x86_-DCMAKE_CXX_STANDARD=17.txt
______3.9-dbg__deadsnakes______Valgrind_____x64.txt
______3.9_____macos-latest_____x64.txt
______3.9_____MSVC_2019__Debug______x86_-DCMAKE_CXX_STANDARD=20.txt
______3.9_____MSVC_2019_____x86_-DCMAKE_CXX_STANDARD=20.txt
______3.9_____MSVC_2022_C++20_____x64.txt
______3.9_____ubuntu-latest_____x64.txt
______3.9_____windows-2019_____x64.txt
______3.9_____windows-2022_____x64.txt
______3_____almalinux8_____x64.txt
______3_____almalinux9_____x64.txt
______3_____CentOS7__PGI_22.9_____x64.txt
______3_____centos7_____x64.txt
______3_____GCC_10_____C++17____x64.txt
______3_____GCC_11_____C++20____x64.txt
______3_____GCC_12_____C++20____x64.txt
______3_____GCC_7_____C++11____x64.txt
______3_____GCC_7_____C++17____x64.txt
______3_____GCC_8_____C++14____x64.txt
______3_____GCC_8_____C++17____x64.txt
______3_____ICC_latest_____x64.txt
______3_____windows-latest_____mingw32.txt
______3_____windows-latest_____mingw64.txt
______pypy-3.7_____macos-latest_____x64.txt
______pypy-3.7_____ubuntu-latest_____x64.txt
______pypy-3.7_____windows-2022_____x64.txt
______pypy-3.8_____macos-latest_____x64.txt
______pypy-3.8_____ubuntu-latest_____x64_-DPYBIND11_FINDPYTHON=ON.txt
______pypy-3.8_____windows-2022_____x64.txt
______pypy-3.9_____macos-latest_____x64.txt
______pypy-3.9_____ubuntu-latest_____x64.txt
______pypy-3.9_____windows-2022_____x64.txt

rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Nov 6, 2022
…x>` everywhere.

From PR pybind#4316 we know that types in the unnamed namespace in different translation units do not compare equal, as desired.

But do types in named namespaces compare equal, as desired?
@rwgk rwgk changed the title [Experiment] std::type_index & unnamed namespace [Experiment] std::type_index & un/named namespace Nov 7, 2022
… work around the same issue as described under PR pybind#4054.
@rwgk
Copy link
Collaborator Author

rwgk commented Nov 7, 2022

Here is the other half of the truth, in addition to information in the comment above:

The CI results for the current commit are here (everything succeeded):

https://github.com/pybind/pybind11/actions/runs/3407243166/jobs/5666693081

The name of the "Download log archive" file: logs_24772.zip

Results of inspecting the logs are below.

Summary:

  • gcc std::type_index is universally working as desired.
  • MSVC std::type_index is universally working as desired.
  • clang std::type_index for types in named namespaces works as desired everywhere except macOS.
  • clang std::type_index for types in unnamed namespaces does not work as desired anywhere. Note that this is still true for clang 15.0.4 and clang 16.0.0.
$ grep 'std::type_index-NE-BAD' *.txt | sed 's/^[1-9][0-9]*//' | cutniq | sort
______3.10_____macos-latest_____x64.txt
______3.11_____macos-latest_____x64.txt
______3.6_____macos-latest_____x64.txt
______3.9_____macos-latest_____x64.txt
______pypy-3.7_____macos-latest_____x64.txt
______pypy-3.8_____macos-latest_____x64.txt
______pypy-3.9_____macos-latest_____x64.txt
$ grep 'std::type_index-EQ-GOOD' *.txt | sed 's/^[1-9][0-9]*//' | cutniq | sort
______3.10_____CUDA_11.7_____Ubuntu_22.04.txt
______3.10_____ubuntu-latest_____x64.txt
______3.10_____windows-2022_____x64.txt
______3.11__deadsnakes______x64.txt
______3.11_____ubuntu-latest_____x64.txt
______3.11_____windows-2022_____x64.txt
______3.6_____MSVC_2019_____x86.txt
______3.6_____ubuntu-latest_____x64_-DPYBIND11_FINDPYTHON=ON_-DCM.txt
______3.6_____windows-2019_____x64_-DPYBIND11_FINDPYTHON=ON.txt
______3.6_____windows-2022_____x64.txt
______3.7_____Debian_____x86______Install.txt
______3.7_____MSVC_2019_____x86_-DCMAKE_CXX_STANDARD=14.txt
______3.8_____MSVC_2019__Debug______x86_-DCMAKE_CXX_STANDARD=17.txt
______3.8_____MSVC_2019_____x86_-DCMAKE_CXX_STANDARD=17.txt
______3.9-dbg__deadsnakes______Valgrind_____x64.txt
______3.9_____MSVC_2019__Debug______x86_-DCMAKE_CXX_STANDARD=20.txt
______3.9_____MSVC_2019_____x86_-DCMAKE_CXX_STANDARD=20.txt
______3.9_____MSVC_2022_C++20_____x64.txt
______3.9_____ubuntu-latest_____x64.txt
______3.9_____windows-2019_____x64.txt
______3.9_____windows-2022_____x64.txt
______3_____almalinux8_____x64.txt
______3_____almalinux9_____x64.txt
______3_____CentOS7__PGI_22.9_____x64.txt
______3_____centos7_____x64.txt
______3_____Clang_10_____C++17_____x64.txt
______3_____Clang_10_____C++20_____x64.txt
______3_____Clang_11_____C++20_____x64.txt
______3_____Clang_12_____C++20_____x64.txt
______3_____Clang_13_____C++20_____x64.txt
______3_____Clang_14_____C++20_____x64.txt
______3_____Clang_3.6_____C++11_____x64.txt
______3_____Clang_3.7_____C++11_____x64.txt
______3_____Clang_3.9_____C++11_____x64.txt
______3_____Clang_5_____C++14_____x64.txt
______3_____Clang_7_____C++11_____x64.txt
______3_____Clang_9_____C++11_____x64.txt
______3_____Clang_dev_____C++11_____x64.txt
______3_____GCC_10_____C++17____x64.txt
______3_____GCC_11_____C++20____x64.txt
______3_____GCC_12_____C++20____x64.txt
______3_____GCC_7_____C++11____x64.txt
______3_____GCC_7_____C++17____x64.txt
______3_____GCC_8_____C++14____x64.txt
______3_____GCC_8_____C++17____x64.txt
______3_____ICC_latest_____x64.txt
______3_____windows-latest_____mingw32.txt
______3_____windows-latest_____mingw64.txt
______pypy-3.7_____ubuntu-latest_____x64.txt
______pypy-3.7_____windows-2022_____x64.txt
______pypy-3.8_____ubuntu-latest_____x64_-DPYBIND11_FINDPYTHON=ON.txt
______pypy-3.8_____windows-2022_____x64.txt
______pypy-3.9_____ubuntu-latest_____x64.txt
______pypy-3.9_____windows-2022_____x64.txt

rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Nov 7, 2022
@rwgk rwgk closed this Nov 7, 2022
rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Nov 9, 2022
@rwgk
Copy link
Collaborator Author

rwgk commented Nov 9, 2022

Here is another result, for Windows clang (obtained under PR #4321, with the diff from this PR patched in):

SKIPPED [1] test_exc_named_namespace_a.py:10: std::type_index-EQ-GOOD
SKIPPED [1] test_unnamed_namespace_a.py:14: std::type_index-NE-GOOD

Even though clang is used, std::type_index-based type comparisons work as desired, for types in named and unnamed namespaces.

rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Nov 9, 2022
rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Nov 10, 2022
@rwgk
Copy link
Collaborator Author

rwgk commented Nov 10, 2022

Here is another result, for macOS brew install llvm clang (obtained under PR #4324, with the diff from this PR patched in):

SKIPPED [1] test_exc_named_namespace_a.py:14: std::type_index-NE-BAD
SKIPPED [1] test_unnamed_namespace_a.py:14: std::type_index-NE-GOOD

Slightly related side note:

XFAIL test_exceptions.py::test_cross_module_exception_translator

This is even with #define PYBIND11_EXPORT_EXCEPTION PYBIND11_EXPORT in pybind11/detail/common.h.

rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Nov 10, 2022
rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Nov 10, 2022
rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Nov 10, 2022
rwgk pushed a commit that referenced this pull request Apr 25, 2023
…erywhere **except when libc++ is in use** (#4319)

* Try using `std::hash<std::type_index>`, `std::equal_to<std::type_index>` everywhere.

From PR #4316 we know that types in the unnamed namespace in different translation units do not compare equal, as desired.

But do types in named namespaces compare equal, as desired?

* Revert "Try using `std::hash<std::type_index>`, `std::equal_to<std::type_index>` everywhere."

This reverts commit a06949a.

* Use "our own name-based hash and equality functions" for `std::type_index` only under macOS, based on results shown under #4316 (comment)

* Patch in PR #4313: Minimal reproducer for clash when binding types defined in the unnamed namespace.

* test_unnamed_namespace_b xfail for clang

* `PYBIND11_INTERNALS_VERSION 5`

* Add a note to docs/classes.rst

* For compatibility with Google-internal testing, test_unnamed_namespace_a & test_unnamed_namespace_b need to work when imported in any order.

* Trying "__GLIBCXX__ or Windows", based on observations from Google-internal testing.

* Try _LIBCPP_VERSION

* Account for libc++ behavior in tests and documentation.

* Adjust expectations for Windows Clang (and make code less redundant).

* Add WindowsClang to ci.yml

Added block transferred from PR #4321

* Add clang-latest to name that appears in the GitHub Actions web view.

* Tweak the note in classes.rst again.

* Add `pip install --upgrade pip`, Show env, cosmetic changes

Already tested under PR #4321

* Add macos_brew_install_llvm to ci.yml

Added block transferred from PR #4324

* `test_cross_module_exception_translator` xfail 'Homebrew Clang'

* Revert back to base version of .github/workflows/ci.yml (the ci.yml changes were merged under #4323 and #4326)

* Fixes for ruff

* Make updated condition in internals.h dependent on ABI version.

* Remove PYBIND11_TEST_OVERRIDE when testing with PYBIND11_INTERNALS_VERSION=10000000

* Selectively exercise cmake `-DPYBIND11_TEST_OVERRIDE`: ubuntu, macos, windows

Extra work added to quick jobs, based on timings below, to not increase the GHA start-to-last-job-finished time.

```
Duration
^              Number of pytest runs
^              ^ Job identifier
^              ^ ^
0:03:48.024227 1 1___3___Clang_3.6___C++11___x64.txt
0:03:58.992814 1 2___3___Clang_3.7___C++11___x64.txt
0:04:25.758942 1 1___3.7___Debian___x86____Install.txt
0:04:50.148276 1 4___3___Clang_7___C++11___x64.txt
0:04:55.784558 1 13___3___Clang_15___C++20___x64.txt
0:04:57.048754 1 6___3___Clang_dev___C++11___x64.txt
0:05:00.485181 1 7___3___Clang_5___C++14___x64.txt
0:05:03.744964 1 2___3___almalinux8___x64.txt
0:05:06.222752 1 5___3___Clang_9___C++11___x64.txt
0:05:11.767022 1 2___3___GCC_7___C++17__x64.txt
0:05:18.634930 1 2___3.11__deadsnakes____x64.txt
0:05:22.810995 1 1___3___GCC_7___C++11__x64.txt
0:05:25.275317 1 12___3___Clang_14___C++20___x64.txt
0:05:32.058174 1 5___3___GCC_10___C++17__x64.txt
0:05:39.381351 1 7___3___GCC_12___C++20__x64.txt
0:05:40.502252 1 8___3___Clang_10___C++17___x64.txt
0:05:59.344905 1 3___3___Clang_3.9___C++11___x64.txt
0:06:10.825147 1 6___3___GCC_11___C++20__x64.txt
0:06:20.655443 1 3___3___almalinux9___x64.txt
0:06:22.472061 1 3___3___GCC_8___C++14__x64.txt
0:06:42.647406 1 11___3___Clang_13___C++20___x64.txt
0:06:53.352720 1 1___3.10___CUDA_11.7___Ubuntu_22.04.txt
0:07:07.357801 1 2___3.7___MSVC_2019___x86_-DCMAKE_CXX_STANDARD=14.txt
0:07:09.057603 1 1___3___centos7___x64.txt
0:07:15.546282 1 1___3.8___MSVC_2019__Debug____x86_-DCMAKE_CXX_STANDARD=17.txt
0:07:22.566022 1 4___3___GCC_8___C++17__x64.txt
0:08:13.592674 1 2___3.9___MSVC_2019__Debug____x86_-DCMAKE_CXX_STANDARD=20.txt
0:08:16.422768 1 9___3___Clang_11___C++20___x64.txt
0:08:21.168457 1 3___3.8___MSVC_2019___x86_-DCMAKE_CXX_STANDARD=17.txt
0:08:27.129468 1 10___3___Clang_12___C++20___x64.txt
0:09:35.045470 1 1___3.10___windows-latest___clang-latest.txt
0:09:57.361843 1 1___3.9___MSVC_2022_C++20___x64.txt
0:10:35.187767 1 1___3.6___MSVC_2019___x86.txt
0:11:14.691200 4 2___3.9___ubuntu-20.04___x64.txt
0:11:37.701167 1 1_macos-latest___brew_install_llvm.txt
0:11:38.688299 4 4___3.11___ubuntu-20.04___x64.txt
0:11:52.720216 1 4___3.9___MSVC_2019___x86_-DCMAKE_CXX_STANDARD=20.txt
0:13:23.456591 4 6___pypy-3.8___ubuntu-20.04___x64_-DPYBIND11_FINDPYTHON=ON.txt
0:13:25.863592 2 1___3___ICC_latest___x64.txt
0:13:32.411758 3 9___3.9___windows-2022___x64.txt
0:13:45.473377 4 3___3.10___ubuntu-20.04___x64.txt
0:13:55.366447 4 5___pypy-3.7___ubuntu-20.04___x64.txt
0:13:57.969502 3 10___3.10___windows-2022___x64.txt
0:14:19.837475 3 11___3.11___windows-2022___x64.txt
0:14:33.316770 4 1___3.6___ubuntu-20.04___x64_-DPYBIND11_FINDPYTHON=ON_-DCMA.txt
0:15:34.449278 4 22___3.6___windows-2019___x64_-DPYBIND11_FINDPYTHON=ON.txt
0:16:25.189055 2 1___3.9-dbg__deadsnakes____Valgrind___x64.txt
0:17:20.956667 4 15___3.6___macos-latest___x64.txt
0:17:27.513891 4 23___3.9___windows-2019___x64.txt
0:17:58.783286 3 8___3.6___windows-2022___x64.txt
0:18:25.917828 4 7___pypy-3.9___ubuntu-20.04___x64.txt
0:19:17.399820 3 13___pypy-3.8___windows-2022___x64.txt
0:19:45.002122 3 12___pypy-3.7___windows-2022___x64.txt
0:20:03.201926 4 16___3.9___macos-latest___x64.txt
0:20:15.415178 4 17___3.10___macos-latest___x64.txt
0:20:20.263216 4 20___pypy-3.8___macos-latest___x64.txt
0:20:31.998226 3 1___3___windows-latest___mingw64.txt
0:20:40.812286 4 18___3.11___macos-latest___x64.txt
0:22:47.714749 4 19___pypy-3.7___macos-latest___x64.txt
0:23:04.435859 3 2___3___windows-latest___mingw32.txt
0:25:48.719597 3 14___pypy-3.9___windows-2022___x64.txt
0:26:01.211688 4 21___pypy-3.9___macos-latest___x64.txt
0:28:19.971015 1 1___3___CentOS7__PGI_22.9___x64.txt
```

* Update skipif for Python 3.12a7 (the WIP needs to be handled in a separate PR).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant