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

Effectively undoing #2999, to see what platforms are affected. #4284

Closed
wants to merge 1 commit into from

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Oct 25, 2022

Description

EXPERIMENTAL, related to #4105, #2999, #4283

Suggested changelog entry:

@rwgk
Copy link
Collaborator Author

rwgk commented Oct 31, 2022

The only 4 (of 75) jobs that failed were (https://github.com/pybind/pybind11/actions/runs/3318113387/jobs/5481692114):

  • CI / 🐍 3.6 • macos-latest • x64 (pull_request) Failing after 8m
  • CI / 🐍 3.9 • macos-latest • x64 (pull_request) Failing after 12m
  • CI / 🐍 3.10 • macos-latest • x64 (pull_request) Failing after 8m
  • CI / 🐍 3.11-dev • macos-latest • x64 (pull_request) Failing after 13m

Note that pypy macos-latest succeeded, but only because test_cross_module_exception_translator() in test_exceptions.py is marked as xfail.

Summary: All macos-latest jobs need PYBIND11_EXPORT_EXCEPTION, all other jobs do not.

rwgk pushed a commit to rwgk/pybind11 that referenced this pull request Oct 31, 2022
Background: pybind#2999, pybind#4105, pybind#4283, pybind#4284

In a nutshell:

* Only macOS actually needs `PYBIND11_EXPORT_EXCEPTION` (pybind#4284).

* Evidently (pybind#4283), under macOS `PYBIND11_EXPORT_EXCEPTION` does not run the risk of introducing ODR violations,

* but evidently (pybind#4283) under Linux it does, in the presumably rare/unusual situation that `RTLD_GLOBAL` is used.

* Windows does no have the equivalent of `RTLD_GLOBAL`, therefore `PYBIND11_EXPORT_EXCEPTION` has no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.
henryiii pushed a commit that referenced this pull request Oct 31, 2022
…#4298)

Background: #2999, #4105, #4283, #4284

In a nutshell:

* Only macOS actually needs `PYBIND11_EXPORT_EXCEPTION` (#4284).

* Evidently (#4283), under macOS `PYBIND11_EXPORT_EXCEPTION` does not run the risk of introducing ODR violations,

* but evidently (#4283) under Linux it does, in the presumably rare/unusual situation that `RTLD_GLOBAL` is used.

* Windows does no have the equivalent of `RTLD_GLOBAL`, therefore `PYBIND11_EXPORT_EXCEPTION` has no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.
@rwgk rwgk closed this Oct 31, 2022
rwgk pushed a commit that referenced this pull request Nov 18, 2022
…#4298)

Background: #2999, #4105, #4283, #4284

In a nutshell:

* Only macOS actually needs `PYBIND11_EXPORT_EXCEPTION` (#4284).

* Evidently (#4283), under macOS `PYBIND11_EXPORT_EXCEPTION` does not run the risk of introducing ODR violations,

* but evidently (#4283) under Linux it does, in the presumably rare/unusual situation that `RTLD_GLOBAL` is used.

* Windows does no have the equivalent of `RTLD_GLOBAL`, therefore `PYBIND11_EXPORT_EXCEPTION` has no practical benefit, on the contrary, noisy warning suppression pragmas are needed, therefore it is best left empty.
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