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

Suggested fixes for variant.h #24

Open
barracuda156 opened this issue Sep 29, 2023 · 5 comments
Open

Suggested fixes for variant.h #24

barracuda156 opened this issue Sep 29, 2023 · 5 comments

Comments

@barracuda156
Copy link
Contributor

@jagerman Returning to this finally.

  1. Sorry, I missed it somehow earlier: we got an unneeded duplicate condition here:
    #ifdef __APPLE__
    #include <AvailabilityMacros.h>
    #if defined(__APPLE__) && (MAC_OS_X_VERSION_MIN_REQUIRED < 101400)
    #define BROKEN_APPLE_VARIANT
    #endif
    #endif

Should be just:

#ifdef __APPLE__
#include <AvailabilityMacros.h>
#if MAC_OS_X_VERSION_MIN_REQUIRED < 101400
#define BROKEN_APPLE_VARIANT
#endif
#endif

This is non-functional, but the code will be neater.

  1. variant.h does not seem to encounter any issues when GCC is used: variant.h: minor fixes for Darwin #16 (comment)
    I have also verified that with oxen-encoding 1.0.8 now.

Should I test something else, or we can make a “broken” fallback conditional on Clang?

@jagerman
Copy link
Member

Does the gcc build use its own stdlibc++, or is it using the system libc++? (The problem is specific to AppleClang + apple's oft-broken libc++).

jagerman added a commit that referenced this issue Sep 29, 2023
@jagerman
Copy link
Member

In theory, #if defined(__APPLE__) && defined(__clang__) && defined(_LIBCPP_VERSION) might be enough here.

@barracuda156
Copy link
Contributor Author

@jagerman GCC normally (by default) uses libstdc++.

For clang, it may work with libc++, but we will need to use LLVM clang with a newer runtime libs, and I am not sure how to write a logic via macros for that.
(I know how to write the logic for compiler and runtime choice in the portfile, but can test it a bit later. GCC works fine though, that is tested.)

@barracuda156
Copy link
Contributor Author

In theory, #if defined(__APPLE__) && defined(__clang__) && defined(_LIBCPP_VERSION) might be enough here.

Technically, we have Apple clang with system runtime and LLVM clang with system runtime or its own runtime. Recent LLVM clang with its own libc++ should work, and they can be used at least back to 10.5.
Do we have a macro to differentiate between these?

jagerman added a commit that referenced this issue Nov 18, 2023
@jagerman
Copy link
Member

With f6172d5 this is now:

#if defined(__APPLE__) && defined(__clang__)

and the redundant extra __APPLE__ check was fixed in 096b289.

Do we have a macro to differentiate between these?

I don't know; the last time I looked at this there was no simple compiler predef to disambiguate, but if you know of something I'd happily put it in.

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

No branches or pull requests

2 participants