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

Constexpr CRC for C++23 #907

Draft
wants to merge 15 commits into
base: pull-request/#907-Constexpr-CRC-for-C++23
Choose a base branch
from

Conversation

jaskij
Copy link
Contributor

@jaskij jaskij commented Jun 25, 2024

This ended up a bit bigger than I originally intended, but all the tests pass.

What's added:

  • basic C++23 support wired: CI, ETL_CONSTEXPR23, CI runs for C++23
  • CRC checksums should be fully constexpr for C++23 builds

C++23

Since my feature requires C++23, I added basic support for it, including:

  • ETL_CONSTEXPR23
  • fixing tests not building because of std::aligned_storage deprecation
  • very basic C++23 support detection (which fails for GCC 13)
  • CI

Constexpr CRC

I wanted to created some binary headers for use with a bootloader, and for those headers to be guarded with CRC. Since I'm using ETL's implementation, I updated the library to actually allow using CRCs in a constant evaluated context.

The requirement for C++23 comes from P2647R1, which permits putting constexpr on etl::crc_table::add(). The limitation being the static part of the actual table. This could probably be relaxed, but I don't exactly see how at the moment, not with support for C++98 or C++03.

Copy link

Review changes with SemanticDiff.

@jaskij
Copy link
Contributor Author

jaskij commented Jun 25, 2024

Sorry for bundling it like this, the PR was written in several hours of sleep-deprivation-fueled-mania. I can split the C++23 support later on.

I can also tell already that the check for C++23 will probably need to be refined in the future.

@jwellbelove jwellbelove changed the base branch from master to pull-request/#907-Constexpr-CRC-for-C++23 June 27, 2024 11:13
@jaskij
Copy link
Contributor Author

jaskij commented Jun 27, 2024

That last push should fix the C++11 builds (my fault for using ETL_CONSTEXPR instead of ETL_CONSTEXPR14).

Right now I have absolutely no clue though what's going on in those C++23 test on clang. The include stack would point to something in UnitTest++?

[  8%] Building CXX object test/CMakeFiles/etl_tests.dir/test_algorithm.cpp.o
In file included from /home/runner/work/etl/etl/test/test_algorithm.cpp:29:
In file included from /home/runner/work/etl/etl/test/unit_test_framework.h:32:
In file included from /home/runner/work/etl/etl/test/UnitTest++/UnitTest++.h:1:
In file included from /home/runner/work/etl/etl/test/UnitTest++/UnitTestPP.h:5:
In file included from /home/runner/work/etl/etl/test/UnitTest++/TestMacros.h:7:
In file included from /home/runner/work/etl/etl/test/UnitTest++/ExecuteTest.h:8:
In file included from /home/runner/work/etl/etl/test/UnitTest++/MemoryOutStream.h:9:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/sstream:40:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/istream:40:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/ios:44:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/ios_base.h:41:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/locale_classes.h:40:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/string:67:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/memory_resource.h:41:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/uses_allocator_args.h:39:

@jwellbelove
Copy link
Contributor

It looks like the clang checks are failing at the point where clang is being upgraded to version 17.
The messages seem to indicate that it is already at v17.

@jaskij
Copy link
Contributor Author

jaskij commented Jul 2, 2024

Sorry about the multiple pushes, hopefully it's not much of an issue.

I don't have a Ubuntu system on hand, but removing the installation and changing the builds to clang/clang++ 18 should work, hopefully

@jaskij jaskij mentioned this pull request Jul 2, 2024
@jaskij jaskij marked this pull request as draft July 3, 2024 02:07
@jaskij
Copy link
Contributor Author

jaskij commented Jul 3, 2024

In light of #912 , I marked this PR as a draft. My current plan is:

  • Improve ci #912
  • general C++23 enablement work as a separate PR (right now it's in here)
  • rebase or rewrite this PR

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