clar: introduce type-safe integer comparisons #117
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
@ethomson Would you mind doing a review of this MR? 🙏 |
pks-t
force-pushed
the
pks-integer-comparisons
branch
from
9e99603 to
e818e71
Compare
phillipwood
reviewed
Dec 1, 2025
phillipwood
reviewed
Dec 1, 2025
phillipwood
reviewed
Dec 1, 2025
Contributor
|
@pks-t Thanks for working on this, it will be a very welcome improvement. I've left a couple of small comments but this all looks sensible to me. |
pks-t
force-pushed
the
pks-integer-comparisons
branch
from
e818e71 to
eeeeca8
Compare
Member
Author
|
@phillipwood Thanks for your review! I've rebased the PR now to fix conflicts and have applied your feedback, except for the |
Introduce a portable wrapper for `vsnprintf()`. This function will be used in subsequent commits.
While we already have `clar_fail()`, this function does not allow the caller to provide a printf(3p)-style formatting string. This makes it somewhat hard at times to give a proper explanation _why_ a specific test has failed. Introduce `clar_failf()` to plug this gap. Note that the core of this new functionality is implemented in `clar__failv()`, which receives a `va_list` as input. This indirection isn't needed right now, but we will add more callers in subsequent commits.
The macros we have to assert the state of integers are lacking due to
multiple reasons:
- We explicitly cast the values to `int`, which causes problems in
case the values do not fit into an `int`. Furthermore, this hides
issues in case one accidentally passes the wrong type to this macro.
- We only have macros to compare integers for equality. Notably
lacking are constructs to compare for non-equality, like "less
than" or "less or equal".
- We only have macros to compare _signed_ integers, but lack macros to
check for _unsigned_ macros.
Fix this issue by introducing `clar__assert_compare_i()` as well as an
equivalent for unsigned types, `clar__assert_compare_u()`. These macros:
- Get `intmax_t` and `uintmax_t` as input, respectively, which allows
us to get rid of the explicit casts. Instead, the compiler can now
verify types for us and print warnings when there is an incompatible
type.
- Get an enum as input for the various different comparisons. Like
this we don't only support equality checks, but also all the other
checks one would typically expect.
Adapt existing macros to use `clar__assert_compare_i()`. Furthermore,
introduce new macros that supersede the older variants and which allow
the caller to perform integer comparisons.
pks-t
force-pushed
the
pks-integer-comparisons
branch
from
eeeeca8 to
06f0c35
Compare
phillipwood
approved these changes
Dec 4, 2025
Contributor
phillipwood
left a comment
phillipwood
left a comment
There was a problem hiding this comment.
@pks-t The range-diff looks good, thanks again for adding these assertions.
Member
Author
|
Thanks, @phillipwood! Let's merge this then. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The macros we have to assert the state of integers are lacking due to
multiple reasons:
We explicitly cast the values to
int, which causes problems incase the values do not fit into an
int. Furthermore, this hidesissues in case one accidentally passes the wrong type to this macro.
We only have macros to compare integers for equality. Notably
lacking are constructs to compare for non-equality, like "less
than" or "less or equal".
We only have macros to compare signed integers, but lack macros to
check for unsigned macros.
Fix this issue by introducing
clar__assert_compare_i()as well as anequivalent for unsigned types,
clar__assert_compare_u(). These macros:Get
intmax_tanduintmax_tas input, respectively, which allowsus to get rid of the explicit casts. Instead, the compiler can now
verify types for us and print warnings when there is an incompatible
type.
Get an enum as input for the various different comparisons. Like
this we don't only support equality checks, but also all the other
checks one would typically expect.
Adapt existing macros to use
clar__assert_compare_i(). Furthermore,introduce new macros that supersede the older variants and which allow
the caller to perform integer comparisons.