Skip to content

Added std::endian enum#66

Open
Chi-Iroh wants to merge 11 commits intolackhole:mainfrom
Chi-Iroh:endian
Open

Added std::endian enum#66
Chi-Iroh wants to merge 11 commits intolackhole:mainfrom
Chi-Iroh:endian

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

@Chi-Iroh Chi-Iroh marked this pull request as draft November 4, 2025 15:11
@Chi-Iroh
Copy link
Contributor Author

Chi-Iroh commented Nov 5, 2025

Hey @lackhole, I'm back and I think I fixed all the issues you mentioned.
We'll see how the test go, but I tested the non-CMake config I made with macros (inspired by Boost, with some additions by me) on multiple platforms (an Android phone, 2 Mac (Apple Clang 13 & 17), an Ubuntu (gcc 14) and a Windows 10 (gcc 15)).
Hope it's better now, but let me know if you'd like extra changes.

@Chi-Iroh Chi-Iroh marked this pull request as ready for review November 5, 2025 10:41
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