Skip to content

Conversation

lemire
Copy link
Member

@lemire lemire commented Jun 4, 2025

Before submitting this PR, run tools/clang-format.sh on your changes. See the "Contributing" section in the README for details.

Fixes #711

@lemire lemire requested a review from SLieve June 4, 2025 13:45
@lemire lemire requested a review from Copilot June 4, 2025 14:04
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces direct uses of C’s _Static_assert in the ART code with a portable CROARING_STATIC_ASSERT macro and defines that macro to map to static_assert in C++ or _Static_assert in C.

  • Added CROARING_STATIC_ASSERT macro in portability.h to unify C/C++ assertions.
  • Updated four alignment checks and one size check in art.c to use the new macro.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/art/art.c Replaced _Static_assert calls with CROARING_STATIC_ASSERT
include/roaring/portability.h Defined CROARING_STATIC_ASSERT macro for C/C++

Comment on lines +592 to +593
#else
#define CROARING_STATIC_ASSERT(x, y) _Static_assert(x, y)
Copy link
Preview

Copilot AI Jun 4, 2025

Choose a reason for hiding this comment

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

Guard the C branch of this macro with a check for C11 support (e.g., __STDC_VERSION__ >= 201112L) so that older C compilers without _Static_assert don’t break the build.

Suggested change
#else
#define CROARING_STATIC_ASSERT(x, y) _Static_assert(x, y)
#elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L)
#define CROARING_STATIC_ASSERT(x, y) _Static_assert(x, y)
#else
#define CROARING_STATIC_ASSERT(x, y) /* static assert not supported */

Copilot uses AI. Check for mistakes.

Copy link
Contributor

@SLieve SLieve left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@lemire lemire merged commit db33cd3 into master Jun 4, 2025
33 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.

Compilation issue with G++ 12 to 14
2 participants