Avoid using _Bool in public headers for the sake of C++ compatibility #12787
Conversation
kit-ty-kate
left a comment
There was a problem hiding this comment.
I believe a similar change needs to happen in the 5.1 branch as https://github.com/ocaml/ocaml/pull/12734/files#diff-92e5994819665dce8d9ae0342af2dbb1ff3cd11666eb080873f54c2cc04a5ca3R138 introduced a use of _Bool as well.
|
This other occurrence is under a I checked and the current modification is the only occurrence of |
kayceesrk
left a comment
There was a problem hiding this comment.
LGTM.
Do we have tests in the testsuite that fail if the RET_FROM_C_CALL macro does not work as expected?
|
Even if the remaining |
|
We test that the poll happens when it should. We do not test that no poll happens when it should not (and it would ask some effort to devise one). I'm a proponent of using bool inside the runtime, which sometimes leads to using _Bool inside internal headers. All the more as the situation will resolve itself in the long term with C23. A test that would be nice to have in the CI is that C++ compilers understand public OCaml headers. |
Headers must be compatible with C++ so they cannot refer to _Bool. We replace _Bool for Caml_state->action_pending with intnat where only the least-significant byte is taken into account.
Fixes: #12772. See discussion there regarding the choice for s390x.
Precheck passes: https://ci.inria.fr/ocaml/job/precheck/915/