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

Added std::endian enum #66

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Added std::endian enum #66

wants to merge 4 commits into from

Conversation

Chi-Iroh
Copy link
Contributor

Here's C++20 endianness support. Feel free to correct me if my CMake file can be improved, I'm not very familiar with CMake.

@lackhole lackhole self-requested a review November 26, 2024 06:11
@lackhole lackhole added the c++20 label Nov 26, 2024
@lackhole
Copy link
Owner

lackhole commented Dec 1, 2024

Hi @Chi-Iroh, thank you for the contribution. Can you modify your code to provide a more robustness?

  • Static assert is in wrong format. Use following: static_assert(sizeof(char) == 1, "");
  • Forward compile options when running endianness compilation/runtime check (i.e., ARM can use both)
  • Add a user-settable option for PREVIEW_ENDIAN and don't run configuration if set (i.e., ARM can use both)
  • Use program return code rather than relying on read/write (return 0, 1, 2, ...)
  • Do not use add_compile_definition. STL-Preview uses configure_file. See cmake_config.h.in and RunConfiguration.cmake
  • Write a fallback case for non-CMake configuration. See preview/config.h

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants