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

Allow compile time CRC calculation #1016

Conversation

Zob314
Copy link

@Zob314 Zob314 commented Jan 16, 2025

Adds constexpr to CRC, allowing compile time computation of CRC values.

I also added constexpr specific tests, but I'm really not sure if every CRC type needs a constexpr test. I'm open to suggestions.

Tested in c++11, c++14, c++17, and c++20 with gcc

All the tests were added via Regex pattern matching, so they're all nearly identical.

@jwellbelove jwellbelove changed the base branch from development to pull-request/#1016-Allow-compile-time-CRC-calculation January 20, 2025 16:42
@jwellbelove
Copy link
Contributor

The tests fail for C++03.

@@ -107,7 +107,7 @@ namespace etl
//*************************************************************************
/// Default constructor.
//*************************************************************************
frame_check_sequence()
ETL_CONSTEXPR14 frame_check_sequence() : frame_check{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI fails for C++03 syntax check.
You will have to change frame_check{} to frame_check() for C++03 compatibility.

@@ -241,16 +241,21 @@ namespace etl
struct crc_table<TAccumulator, Accumulator_Bits, Chunk_Bits, Chunk_Mask, Polynomial, Reflect, 4U>
{
//*************************************************************************
#if !ETL_USING_CPP11 || defined(ETL_FORCE_NO_ADVANCED_CPP)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is ETL_FORCE_NO_ADVANCED_CPP supported? It's only in profile.h and some tests, and it seems to break a few things when defined.

I can remove the extra check if it's not supported.

Copy link
Contributor

@jwellbelove jwellbelove Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like ETL_FORCE_NO_ADVANCED_CPP is a redundant macro left over from some early compile options. It's not even documented.
I will mark them for removal.

@jwellbelove jwellbelove merged commit 14b50c6 into ETLCPP:pull-request/#1016-Allow-compile-time-CRC-calculation Jan 24, 2025
63 checks passed
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.

3 participants