Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Jul 20, 2022

I noticed that we lacked such a thing while working on #17475. It turns
out that system header are technically allowed to redefine macros
without generating a warning, but when I made the SDL header tree into a
normal -I path I noticed that M_PI was being duplicately defined
because we were not defining HAVE_M_PI.

This list of defines is taked from SDL_config_iphoneos.h and a visual
inspection seems to confirm that we do indeed support all the specified
features.

See https://gcc.gnu.org/onlinedocs/cpp/System-Headers.html:
"Macros defined in a system header are immune to a few warnings wherever they are expanded. This immunity is granted on an ad-hoc basis, when we find that a warning generates lots of false positives because of code in macros defined in system headers."

@sbc100 sbc100 requested a review from dschuff July 20, 2022 06:02
@sbc100 sbc100 force-pushed the add_sdl_config branch 2 times, most recently from 9ded74d to 08a70a4 Compare July 20, 2022 06:05
@sbc100 sbc100 enabled auto-merge (squash) July 21, 2022 17:07
@sbc100 sbc100 requested a review from kripken July 21, 2022 19:34
@sbc100
Copy link
Collaborator Author

sbc100 commented Jul 21, 2022

Removed a bunch of defines.

I noticed that we lacked such a thing while working on #17475.  It turns
out that system header are technically allowed to redefine macros
without generating a warning, but when I made the SDL header tree into a
normal `-I` path I noticed that `M_PI` was being duplicately defined
because we were not defining `HAVE_M_PI`.

This list of defines is taked from `SDL_config_iphoneos.h` and a visual
inspection seems to confirm that we do indeed support all the specified
features.
@sbc100 sbc100 merged commit d2e79de into main Jul 22, 2022
@sbc100 sbc100 deleted the add_sdl_config branch July 22, 2022 00:19
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.

3 participants