-
Notifications
You must be signed in to change notification settings - Fork 105
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
Use stdbool.h #118
Use stdbool.h #118
Conversation
source/include/stdbool.readme
Outdated
|
||
#endif | ||
|
||
#define __bool_true_false_are_defined 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the significance of this macro?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's part of the C standard to define this macro in stdbool.h along with the other three: https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdbool.h.html
@@ -0,0 +1,31 @@ | |||
|
|||
#ifndef _STDBOOL_H |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a GitHub Action job to actually run the build check with the stdbool.readme
and stdint.readme
files to validate their definitions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added. Couldn't think of a good name for it though
58c414b
to
78a29b9
Compare
78a29b9
to
af1c88a
Compare
.github/workflows/ci.yml
Outdated
cp source/include/stdint.readme override-include/stdint.h | ||
cmake -S test -B build/ \ | ||
-G "Unix Makefiles" \ | ||
-DCMAKE_BUILD_TYPE=Debug \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is conflicting with the -DNDEBUG
flag passed. Should that flag be passed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just copied the steps of the unittest
job, we do this so that asserts don't show up in coverage. I could remove one or both and it shouldn't really affect anything since we're only testing if it builds without errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the unittest
job should mention the build type as RELEASE
which (I think) would automatically take care of providing the -DNDEBUG
flag to the compiler.
That is beyond the scope of this PR though, so will not block on it.
Description:
Removes the preprocessor check that relies on C99 macros and just includes
stdbool.h
. Astdbool.readme
file, along withstdint.readme
, is provided for compilers that do not provide their respective headers