Use C11/C++11 standard static assertion #12537
Merged
Conversation
This file contains 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
MisterDA
changed the title
|
I can probably convince myself that the first commit is correct, but
will refrain from approving because I don't feel expert enough.
Regarding the second commit, I have to say I don'treally see thepoint
because removing the macro makes itmandatory to specify an empty message
at each call site so I am not convinced we are gaininganything here?
|
Note that the empty message is mandatory in C11/C++11 but optional in C23/C++17. Not using |
|
OK, Iam convinced now, thanks! But jsut as a non-expert so I don't dare
to approve.
|
MisterDA
force-pushed
the
static_assert
branch
from
c13ab0d
to
3d1b637
Compare
xavierleroy
reviewed
Oct 3, 2023
MisterDA
force-pushed
the
static_assert
branch
from
3d1b637
to
2cf4634
Compare
dustanddreams
approved these changes
Oct 4, 2023
Remove CAML_STATIC_ASSERT, use static_assert directly. The assertions were moved to .c files as not to pollute the ocaml user-facing headers with includes of assert.h.
MisterDA
force-pushed
the
static_assert
branch
from
2cf4634
to
4304956
Compare
|
I've squashed the commits. |
xavierleroy
approved these changes
Oct 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
A new step in my quest of pushing usage of standard C/C++ functions in OCaml… let's use the standard static assert mechanism. "old" versions of C/C++ require a message, newer versions don't. Prefer using a keyword instead of a macro, that's one fewer level of indirection. The error message is as clear as it gets.
static_assertdeclaration (since C++11) (cppreference);static_assertdeclaration (since C++11) (msvc).If we consider that all the compilers we support have full C11/C++11 support and are not buggy, we could remove the old code, or even use
static_assertdirectly (as a macro for C11). The last commit does just that. If reviewers think it's too early to include it, I'll remove it from the PR. The macroCAML_STATIC_ASSERTisn't used in any of the opam packages.