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: Remove redundant error code argument from std::filesystem::exist… #1535

Closed
wants to merge 1 commit into from

Conversation

gubatron
Copy link
Contributor

  • Removed the second argument (error_code) from the std::filesystem::exists call in parser.cpp, as the function does not accept an error code parameter in C++17 and later.
  • This resolves a compatibility issue with modern C++ standards.

…s call

- Removed the second argument (error_code) from the std::filesystem::exists call in `parser.cpp`, as the function does not accept an error code parameter in C++17 and later.
- This resolves a compatibility issue with modern C++ standards.

Tested the change to ensure configuration files are correctly checked for existence without affecting error handling logic.
@gubatron
Copy link
Contributor Author

gubatron commented Oct 22, 2024

This was breaking the build for me on macos building with std=c++20 and boost 1.76.0

$ clang --version
Apple clang version 16.0.0 (clang-1600.0.26.3)
Target: arm64-apple-darwin23.5.0
Thread model: posix

@evoskuil
Copy link
Member

Thanks, so you have a successful ARM build? We don’t CI that platform yet.

@evoskuil
Copy link
Member

evoskuil commented Oct 22, 2024

Documentation states that the API is new in C++17, and the change to the second override causes a change from noexcept to throws, which is a problem. https://en.cppreference.com/w/cpp/filesystem/exists

@gubatron
Copy link
Contributor Author

interesting, I couldn't run ./configure with C++17
Yes, building on macos arm64

My goal is to eventually build libbitcoin-node to give you a hand there with lots of low hanging fruit.
Building libbitcoin-network now

@gubatron
Copy link
Contributor Author

ran again configure with C++17 on macos arm, this happens, both here and in libbitcoin-network:

...
checking pkg-config is at least version 0.9.0... yes
checking whether g++ supports C++20 features with -std=c++20... no
checking whether g++ supports C++20 features with +std=c++20... no
checking whether g++ supports C++20 features with -h std=c++20... no
configure: error: *** A compiler with support for C++20 language features is required.

@evoskuil
Copy link
Member

evoskuil commented Oct 22, 2024

C++20 is required, but the API is “since C++17” which includes C++20. In other words, it should be fine when building with C++20. Great to have you onboard!

@evoskuil
Copy link
Member

Also, we would really love to have an macOS ARM build set up in our GitHub Actions CI process!

@evoskuil
Copy link
Member

See: libbitcoin/libbitcoin-node#686

@evoskuil evoskuil closed this Oct 26, 2024
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