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

fix: Disable false warn for GGC 12 bound checking #5355

Merged
merged 1 commit into from
Sep 15, 2024

Conversation

BobbyRBruce
Copy link
Contributor

@BobbyRBruce BobbyRBruce commented Sep 5, 2024

Description

Issue: #5224

The internals.registered_types_py... line in pybind11.h triggers a
false-positive bounds checking warning in GCC 12.

This is discussed in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115824.

The workaround implemented is taken from suggestions then refactored to
use the PYBIND11_WARNING_PUSH and PYBIND11_WARNING_POP MACROS.

Suggested changelog entry:

Disable false-positive GCC 12 Bound Check warning.

include/pybind11/pybind11.h Outdated Show resolved Hide resolved
@BobbyRBruce BobbyRBruce changed the title Add workaround/fix for GGC 12 bound checking error Add warn disable for GGC 12 bound checking error Sep 8, 2024
BobbyRBruce added a commit to BobbyRBruce/gem5 that referenced this pull request Sep 8, 2024
There is a compiler error with GCC 12 discussed here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115824

This Pybind code triggers the bug and was causing our compiler tests to
fail.

To fix gem5 compilation for gcc 12 these warnings/errors have been
suppressed for this code.

This is a copy and paste of:
pybind/pybind11#5355

Change-Id: I9344951ef00d121ea0b609f4faa13dfe09aabb3b
include/pybind11/pybind11.h Outdated Show resolved Hide resolved
@rwgk
Copy link
Collaborator

rwgk commented Sep 11, 2024

(I'll reply quicker this time around. Sorry I didn't see your last commit until now.)

nkrim pushed a commit to nkrim/gem5 that referenced this pull request Sep 11, 2024
There is a compiler error with GCC 12 discussed here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115824

This Pybind code triggers the bug and was causing our compiler tests to
fail.

To fix gem5 compilation for gcc 12 these warnings/errors have been
suppressed for this code.

This is a copy and paste of:
pybind/pybind11#5355

Change-Id: I9344951ef00d121ea0b609f4faa13dfe09aabb3b
Issue: pybind#5224

The `internals.registered_types_py...` line in pybind11.h triggers a
false-positive bounds checking warning in GCC 12.

This is discussed in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=115824.

The workaround implemented is taken from suggestions then refactored to
use the `PYBIND11_WARNING_PUSH` and `PYBIND11_WARNING_POP` MACROS.
@BobbyRBruce
Copy link
Contributor Author

(I'll reply quicker this time around. Sorry I didn't see your last commit until now.)

Your replies are lightening fast compared to my own :). I'm very impressed with Pybind's response to my fix given i'm an outsider to the project.

Thank you for the feedback. I've reduced the comment.

Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@rwgk rwgk merged commit a7910be into pybind:master Sep 15, 2024
78 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Sep 15, 2024
@rwgk
Copy link
Collaborator

rwgk commented Sep 15, 2024

Oh, could you please add a suggestion for a Changelog entry?

See other open PRs for the template.

@BobbyRBruce BobbyRBruce deleted the gcc-12-bound-check-fix branch September 18, 2024 08:05
@BobbyRBruce BobbyRBruce changed the title Add warn disable for GGC 12 bound checking error fix: Disable false warn for GGC 12 bound checking Sep 19, 2024
@BobbyRBruce
Copy link
Contributor Author

Oh, could you please add a suggestion for a Changelog entry?

See other open PRs for the template.

I think I've added this correctly. I just update the PR description here, right?

@henryiii
Copy link
Collaborator

Yes, it gets collected from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changelog Possibly needs a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants