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

Improve SDL_config_unix.h #315

Merged
merged 5 commits into from
Feb 4, 2025
Merged

Improve SDL_config_unix.h #315

merged 5 commits into from
Feb 4, 2025

Conversation

smcv
Copy link
Contributor

@smcv smcv commented Feb 4, 2025

  • SDL_config.h: Assume that Unix platforms define __unix__

    This is believed to be a predefined macro on all reasonable Unix
    platforms in practice (it's certainly predefined on GNU/Linux).
    We can add some || defined(__myplatform__) as necessary, but
    this way is more readable than listing every Unix we know about
    individually.

    https://sourceforge.net/p/predef/wiki/OperatingSystems/ notes that
    "the xlC or the DEC C/C++ compiler" don't define these, but that doesn't
    seem likely to have any practical impact on us.

  • SDL_config.h: Add a #warning if we don't recognise the platform

    SDL_config_minimal.h is rather minimal, and we probably don't want anyone
    to actually be relying on it.

  • SDL_config_unix.h: Describe our basis for assuming that headers exist

    Some of these are Standard C, some are POSIX, and a few are non-standard
    but widely available.

  • SDL_config_unix.h: Only define HAVE_GCC_ATOMICS for gcc or clang

    The gcc atomic operations are a gcc-specific extension, which clang
    implements for compatibility. In the unlikely event that someone compiles
    sdl2-compat with some other compiler, we can't assume their presence.

  • SDL_config_unix.h: Try to use __SIZEOF_POINTER__, if defined

    gcc (and presumably clang) predefines this, and if present, it seems
    reasonable to treat it as authoritative.


As discussed in #229. Compiles successfully on my Debian system, otherwise untested.

smcv added 4 commits February 4, 2025 19:57
This is believed to be a predefined macro on all reasonable Unix
platforms in practice (it's certainly predefined on GNU/Linux).
We can add some `|| defined(__myplatform__)` as necessary, but
this way is more readable than listing every Unix we know about
individually.

https://sourceforge.net/p/predef/wiki/OperatingSystems/ notes that
"the xlC or the DEC C/C++ compiler" don't define these, but that doesn't
seem likely to have any practical impact on us.

Signed-off-by: Simon McVittie <smcv@collabora.com>
SDL_config_minimal.h is rather minimal, and we probably don't want anyone
to actually be relying on it.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Some of these are Standard C, some are POSIX, and a few are non-standard
but widely available.

Signed-off-by: Simon McVittie <smcv@collabora.com>
The gcc atomic operations are a gcc-specific extension, which clang
implements for compatibility. In the unlikely event that someone compiles
sdl2-compat with some other compiler, we can't assume their presence.

Signed-off-by: Simon McVittie <smcv@collabora.com>
gcc (and presumably clang) predefines this, and if present, it seems
reasonable to treat it as authoritative.

Signed-off-by: Simon McVittie <smcv@collabora.com>
@@ -51,6 +51,7 @@
#include "SDL_config_unix.h"
#else
/* This is a minimal configuration just to get SDL running on new platforms. */
#warning SDL_config.h might be incomplete, good luck
Copy link
Contributor

Choose a reason for hiding this comment

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

Is #warning directive universally supported? A #pragma message("...") may also be considered here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe #warning is more widely supported than #pragma message()

@slouken slouken merged commit e6666ee into libsdl-org:main Feb 4, 2025
10 checks passed
@slouken
Copy link
Collaborator

slouken commented Feb 4, 2025

Merged, thanks!

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