Make C stubs more resistant to header structure#55
Make C stubs more resistant to header structure#55emillon wants to merge 2 commits intojanestreet:masterfrom
Conversation
|
cc @rgrinberg ; this replaces #48 and #53. |
|
Isn't this worse? Previously, we'd only open the internals of caml/signals.h, but now we need the internals of all headers. Would it be possible to just import caml/signals.h first with |
|
I don't think that the headers support this kind of modularity: if we do that, I agree with you that it's not completely satisfactory, but:
So I believe it's fine to set it. |
|
It's not "worse" - you're just getting all the internals, which is the unstable part of the OCaml API. Manually declaring the declaration is worse (they do subtly change from time-to-time - linking macros, etc., and even renaming, which you then wouldn't pick up) |
The existing stubs cause a warning on OCaml 5.1 because the declaration of `caml_convert_signal_number` can not be found. The reason for that is that `<caml/signals.h>` is already included by some headers at the beginning of the file, so `CAML_SIGNALS_H` is already set by the time `<caml/signals.h>` is included manually and the extra internal definition are not included. This used to work in previous versions of OCaml because presumably the header structure was different and `<caml/signal.h>` was not included transitively. This fix just sets `CAML_INTERNALS` at the file level; it is not really possible to be granular in setting it. Signed-off-by: Etienne Millon <me@emillon.org>
cb980e0 to
8415508
Compare
|
Enabling CAML_INTERNALAS per header doesn't work, but it's still possible to mitigate the damage. I've moved |
|
Fixed in #58. |
The existing stubs cause a warning on OCaml 5.1 because the declaration of
caml_convert_signal_numbercan not be found.The reason for that is that
<caml/signals.h>is already included by some headers at the beginning of the file, soCAML_SIGNALS_His already set by the time<caml/signals.h>is included manually and the extra internal definition are not included.This used to work in previous versions of OCaml because presumably the header structure was different and
<caml/signal.h>was not included transitively.This fix just sets
CAML_INTERNALSat the file level; it is not really possible to be granular in setting it.