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: gcc uninitialized warnings triggered by unsafe_default_initialized #2203

Closed
wants to merge 1 commit into from

Conversation

mhx
Copy link
Contributor

@mhx mhx commented May 15, 2024

Despite all the FOLLY_*_DISABLE_WARNING, the implementation of unsafe_default_initialized_cv triggers hundreds of -Wuninitialized and -Wmaybe-uninitialized warnings when building with GCC 12, 13, 14. As much as I've tried, it seems impossible to silence the warnings purely by diagnostic pragmas.

This change, however, eliminates all warnings, and the new code should otherwise have the exact same effect as before. At least, I can see that clang / gcc both produce identical code before and after this change when using unsafe_default_initialized to initialize variables.

@facebook-github-bot
Copy link
Contributor

@Orvid has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@mhx
Copy link
Contributor Author

mhx commented Jul 8, 2024

ping

Despite all the `FOLLY_*_DISABLE_WARNING`, the implementation of
`unsafe_default_initialized_cv` triggers hundreds of `-Wuninitialized`
and `-Wmaybe-uninitialized` warnings when building with GCC 12, 13, 14.
As much as I've tried, it seems impossible to silence the warnings
purely by diagnostic pragmas.

This change, however, eliminates all warnings, and the new should
otherwise have the exact same effect as before. At least, I can see
that clang / gcc both produce identical code before and after this
change when using `unsafe_default_initialized` to initialize variables.
@mhx mhx force-pushed the mhx/fix-gcc-uninit-warnings branch from 4d27739 to a7be6f1 Compare July 26, 2024 19:04
@mhx
Copy link
Contributor Author

mhx commented Jul 26, 2024

Updated to a version that mostly resembles the existing bit_cast implementation in folly/lang/Bits.h.

@yfeldblum
Copy link
Contributor

e11e2c2 suppresses this path under gcc.

The approach in this PR could also work, but would have to be restricted to trivial types.

@mhx
Copy link
Contributor Author

mhx commented Aug 8, 2024

Yes, you're correct about the restriction.

I don't mind which way this is ultimately fixed, so thanks for your fix!

@mhx mhx closed this Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants