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

variant.h: minor fixes for Darwin #16

Merged
merged 2 commits into from
May 25, 2023
Merged

Conversation

barracuda156
Copy link
Contributor

This merely fixes macros in use:

  1. Appropriate header to include is AvailabilityMacros.h, which is actually available in every macOS.
  2. MAC_OS_X_VERSION_10_14 is not defined prior to 10.14, so condition is wrong. Numerical constant is to be used.

@jagerman
Copy link
Member

LGTM, thanks for the PR!

@jagerman jagerman merged commit f374150 into oxen-io:dev May 25, 2023
@barracuda156 barracuda156 deleted the darwin branch May 25, 2023 11:53
@barracuda156
Copy link
Contributor Author

LGTM, thanks for the PR!

@jagerman By the way, maybe the code can be fixed instead to be usable on Apple? C++17 is certainly supported and works fine both with GCC and Clang on MacOS, and this includes MacOS at least back to 10.5.8.

@jagerman
Copy link
Member

@jagerman By the way, maybe the code can be fixed instead to be usable on Apple? C++17 is certainly supported and works fine both with GCC and Clang on MacOS, and this includes MacOS at least back to 10.5.8.

It is sort of supported; the core C++17 language is supported, but there chunks of the stl that are not usable when targeting older versions because of missing features in the system libc++ (which Apple apparently never updates on older macOS releases). Off the top of my head, getting a value out of an std::variant or std::any requires 10.13 (although, interestingly, that used to be 10.14 and 10.15, respectively, in some previous versions of Xcode), and touching std::filesystem::path has always required targeting 10.15 or higher (lokinet uses ghc::filesystem instead of std::filesystem on macos specifically because of this).

For example:

#include <filesystem>

int main() {
    std::filesystem::path p;
}
lokinet-mini:~$ c++ -std=c++17 -mmacosx-version-min=10.13 foo.cpp
foo.cpp:4:22: error: 'path' is unavailable: introduced in macOS 10.15
    std::filesystem::path p;
                     ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk/usr/include/c++/v1/__filesystem/path.h:442:24: note: 'path' has been explicitly marked unavailable here
class _LIBCPP_TYPE_VIS path {
                       ^
foo.cpp:4:27: error: 'path' is unavailable: introduced in macOS 10.15
    std::filesystem::path p;
                          ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk/usr/include/c++/v1/__filesystem/path.h:471:25: note: 'path' has been explicitly marked unavailable here
  _LIBCPP_HIDE_FROM_ABI path() noexcept {}
                        ^
foo.cpp:4:27: error: '~path' is unavailable: introduced in macOS 10.15
    std::filesystem::path p;
                          ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX13.3.sdk/usr/include/c++/v1/__filesystem/path.h:505:3: note: '~path' has been explicitly marked unavailable here
  ~path() = default;
  ^
3 errors generated.

@barracuda156
Copy link
Contributor Author

@jagerman For Clangs, Macports can use a newer libc++ than a system one (which is a default for some older systems like 10.6, so you can find ports which work on 10.6 but are broken on, say, 10.9, because manual setting was not added). For GCC, it normally uses its own libstdc++, which solves the problem (and also introduces one occasional malloc complication, but we can sort that).
Has the code been tested with a modern GCC? If there are no clangisms, it should be fixable or usable as is.

@barracuda156
Copy link
Contributor Author

P. S. Filesystem does require a newer libc++ with Clangs, I think our legacysupport can take care of that, though it has to be tested.

@jagerman
Copy link
Member

Has the code been tested with a modern GCC? If there are no clangisms, it should be fixable or usable as is.

Yes; Linux (with gcc) is our primary dev platform. We support anything gcc v8 and up.

@jagerman
Copy link
Member

(I have not, however, tested with gcc on macos, so not sure if there are quirks there)

@barracuda156
Copy link
Contributor Author

Has the code been tested with a modern GCC? If there are no clangisms, it should be fixable or usable as is.

Yes; Linux (with gcc) is our primary dev platform. We support anything gcc v8 and up.

Sounds good. While 10.5 down are at gcc7 at the moment, I have them using gcc12, and anything 10.6+ use gcc12 (though all Intel and aarch64 default to Clangs, GCC being a default on PowerPC and very old Intel).

To test the code (at least a build, to begin with), I just have to comment away a protecting macro, or adjustments elsewhere are required?

@jagerman
Copy link
Member

To test the code (at least a build, to begin with), I just have to comment away a protecting macro, or adjustments elsewhere are required?

I think that's about it; aside from the workarounds for variant/filesystem there shouldn't be anything macos version or clang-dependent.

@barracuda156
Copy link
Contributor Author

To test the code (at least a build, to begin with), I just have to comment away a protecting macro, or adjustments elsewhere are required?

I think that's about it; aside from the workarounds for variant/filesystem there shouldn't be anything macos version or clang-dependent.

I will try it and let you know then. Thanks.

@barracuda156
Copy link
Contributor Author

@jagerman So I just removed all the dedicated code, leaving

#pragma once

#include <variant>

namespace var = std; // Oh look, actual C++17 support

Everything builds neatly, tests pass:

--->  Configuring oxenc
Executing:  cd "/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build" && /opt/local/bin/cmake -G "CodeBlocks - Unix Makefiles" -DCMAKE_BUILD_TYPE=MacPorts -DCMAKE_INSTALL_PREFIX="/opt/local" -DCMAKE_INSTALL_NAME_DIR="/opt/local/lib" -DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr" -DCMAKE_C_COMPILER="$CC" -DCMAKE_CXX_COMPILER="$CXX" -DCMAKE_OBJC_COMPILER="$CC" -DCMAKE_OBJCXX_COMPILER="$CXX" -DCMAKE_POLICY_DEFAULT_CMP0025=NEW -DCMAKE_POLICY_DEFAULT_CMP0060=NEW -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_COLOR_MAKEFILE=ON -DCMAKE_FIND_FRAMEWORK=LAST -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_MAKE_PROGRAM=/usr/bin/make -DCMAKE_MODULE_PATH="/opt/local/share/cmake/Modules" -DCMAKE_PREFIX_PATH="/opt/local/share/cmake/Modules" -DCMAKE_BUILD_WITH_INSTALL_RPATH:BOOL=ON -DCMAKE_INSTALL_RPATH="/opt/local/lib" -Wno-dev -DOXENC_BUILD_DOCS:BOOL=OFF -DOXENC_BUILD_TESTS:BOOL=ON -DOXENC_INSTALL:BOOL=ON -DCMAKE_OSX_ARCHITECTURES="ppc" -DCMAKE_OSX_DEPLOYMENT_TARGET="10.6" -DCMAKE_OSX_SYSROOT="/" /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6 
-- The CXX compiler identification is GNU 12.2.0
-- Checking whether CXX compiler has -isysroot
-- Checking whether CXX compiler has -isysroot - yes
-- Checking whether CXX compiler supports OSX deployment target flag
-- Checking whether CXX compiler supports OSX deployment target flag - yes
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /opt/local/bin/g++-mp-12 - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- oxenc v1.0.6
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE  
-- Configuring done (2.2s)
-- Generating done (0.0s)
CMake Warning:
  Manually-specified variables were not used by the project:

    CMAKE_C_COMPILER
    CMAKE_OBJCXX_COMPILER
    CMAKE_OBJC_COMPILER
    CMAKE_POLICY_DEFAULT_CMP0025
    CMAKE_POLICY_DEFAULT_CMP0060


-- Build files have been written to: /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build
--->  Building oxenc
Executing:  cd "/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build" && /usr/bin/make -j6 -w all VERBOSE=ON 
make: Entering directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build'
/opt/local/bin/cmake -S/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6 -B/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build --check-build-system CMakeFiles/Makefile.cmake 0
/opt/local/bin/cmake -E cmake_progress_start /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/CMakeFiles /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build//CMakeFiles/progress.marks
/usr/bin/make  -f CMakeFiles/Makefile2 all
make[1]: Entering directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build'
/usr/bin/make  -f tests/CMakeFiles/tests.dir/build.make tests/CMakeFiles/tests.dir/depend
make[2]: Entering directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build'
cd /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build && /opt/local/bin/cmake -E cmake_depends "Unix Makefiles" /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6 /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6/tests /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/tests /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/tests/CMakeFiles/tests.dir/DependInfo.cmake --color=
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build'
/usr/bin/make  -f tests/CMakeFiles/tests.dir/build.make tests/CMakeFiles/tests.dir/build
make[2]: Entering directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build'
[ 20%] Building CXX object tests/CMakeFiles/tests.dir/test_endian.cpp.o
cd /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/tests && /opt/local/bin/g++-mp-12  -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6/tests/Catch2/single_include -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6 -pipe -Os -DNDEBUG -I/opt/local/include -D_GLIBCXX_USE_CXX11_ABI=0 -std=c++17 -arch ppc -mmacosx-version-min=10.6 -MD -MT tests/CMakeFiles/tests.dir/test_endian.cpp.o -MF CMakeFiles/tests.dir/test_endian.cpp.o.d -o CMakeFiles/tests.dir/test_endian.cpp.o -c /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6/tests/test_endian.cpp
[ 40%] Building CXX object tests/CMakeFiles/tests.dir/main.cpp.o
cd /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/tests && /opt/local/bin/g++-mp-12  -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6/tests/Catch2/single_include -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6 -pipe -Os -DNDEBUG -I/opt/local/include -D_GLIBCXX_USE_CXX11_ABI=0 -std=c++17 -arch ppc -mmacosx-version-min=10.6 -MD -MT tests/CMakeFiles/tests.dir/main.cpp.o -MF CMakeFiles/tests.dir/main.cpp.o.d -o CMakeFiles/tests.dir/main.cpp.o -c /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6/tests/main.cpp
[ 60%] Building CXX object tests/CMakeFiles/tests.dir/test_encoding.cpp.o
cd /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/tests && /opt/local/bin/g++-mp-12  -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6/tests/Catch2/single_include -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6 -pipe -Os -DNDEBUG -I/opt/local/include -D_GLIBCXX_USE_CXX11_ABI=0 -std=c++17 -arch ppc -mmacosx-version-min=10.6 -MD -MT tests/CMakeFiles/tests.dir/test_encoding.cpp.o -MF CMakeFiles/tests.dir/test_encoding.cpp.o.d -o CMakeFiles/tests.dir/test_encoding.cpp.o -c /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6/tests/test_encoding.cpp
[ 80%] Building CXX object tests/CMakeFiles/tests.dir/test_bt.cpp.o
cd /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/tests && /opt/local/bin/g++-mp-12  -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6/tests/Catch2/single_include -I/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6 -pipe -Os -DNDEBUG -I/opt/local/include -D_GLIBCXX_USE_CXX11_ABI=0 -std=c++17 -arch ppc -mmacosx-version-min=10.6 -MD -MT tests/CMakeFiles/tests.dir/test_bt.cpp.o -MF CMakeFiles/tests.dir/test_bt.cpp.o.d -o CMakeFiles/tests.dir/test_bt.cpp.o -c /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/oxen-encoding-1.0.6/tests/test_bt.cpp
[100%] Linking CXX executable tests
cd /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/tests && /opt/local/bin/cmake -E cmake_link_script CMakeFiles/tests.dir/link.txt --verbose=ON
/opt/local/bin/g++-mp-12 -pipe -Os -DNDEBUG -I/opt/local/include -D_GLIBCXX_USE_CXX11_ABI=0 -arch ppc -mmacosx-version-min=10.6 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/opt/local/lib -Wl,-headerpad_max_install_names CMakeFiles/tests.dir/main.cpp.o CMakeFiles/tests.dir/test_bt.cpp.o CMakeFiles/tests.dir/test_encoding.cpp.o CMakeFiles/tests.dir/test_endian.cpp.o -o tests  -Wl,-rpath,/opt/local/lib 
make[2]: Leaving directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build'
[100%] Built target tests
make[1]: Leaving directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build'
/opt/local/bin/cmake -E cmake_progress_start /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/CMakeFiles 0
make: Leaving directory `/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build'
--->  Testing oxenc
Executing:  cd "/opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build" && /opt/local/var/macports/build/_opt_PPCSnowLeopardPorts_net_oxenc/oxenc/work/build/tests/tests 
===============================================================================
All tests passed (402 assertions in 20 test cases)

This is on 10.6 on PowerPC :)

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.

2 participants