4.10, #9205: move back caml_* to thematic headers #9253
Conversation
to the headers originally defining the corresponding internal variable.
| */ | ||
| #define caml_backtrace_last_exn (Caml_state_field(backtrace_last_exn)) | ||
|
|
||
| /* FIXME: this shouldn't matter anymore. Since OCaml 4.02, non-parameterized |
There was a problem hiding this comment.
Answering a question of @gadmm , those variables are not packed together because the comments in the header details each of them, so it seemed natural to synchronize the documentation with the code. Nevertheless, since there are no other variables defined in-between, it should be easy to add export-control macros around if needed.
|
Patches for |
There was a problem hiding this comment.
Thanks, this is a very polished PR and I do not have any change to request (apart that one should decide once and for all whether @nojb's name is written with or without accents!). In addition, you report successful results of testing the opam repository, and the shape of the fixes to packages it requires looks good. On behalf of all this, it looks good to merge.
| */ | ||
| #define caml_backtrace_last_exn (Caml_state_field(backtrace_last_exn)) | ||
|
|
||
| /* FIXME: this shouldn't matter anymore. Since OCaml 4.02, non-parameterized |
|
Is there a reason for?
Otherwise, the changes look good to me. |
|
Yes, further changes are required due to the change in semantics of allocations, and the proper patch proposed on the Coq github will subsume this one. |
|
Cherry-picked in trunk as 646d304. |
Before a7b5bb6 from ocaml#8713, includes of compatibility.h used to be all guarded by `#ifndef CAML_NAME_SPACE`. The solution at a7b5bb6 required to change two of those guard into `#ifndef CAML_INTERNALS`. The solution finally retained for 4.10 at ocaml#9253 (646d304 in trunk) does not depend on this change anymore, and one of the two guards was changed back into `#ifndef CAML_NAME_SPACE`. This patch changes the second one back as before. Since a second CAML_NAME_SPACE guard is contained in compatibility.h, the potential impact is limited to two of the macros that are not guarded in this way: caml_stat_top_heap_size and caml_stat_heap_size, and limited to programs that somehow came to rely on this in 4.10. Since this patch reverts the situation as in 4.09, the current patch is preferable to include in 4.10, and it is very low risk. No Changes needed.
As suggested by @xavierleroy in #9205, this PR moves back each of the
caml_*currently defined incompatibility.h.This PR provides a compatible API for versions 4.04 to 4.10, at least for programs that were importing internal variables by including the right headers rather than redefining the variables by hand.
After discussion with @kayceesrk , those macros would still be meaningful in the multicore future, since each domain will have its own state; so those macros should not be too ephemeral.
This PR requires a handful of patches to the follwoing opam packages for:
(It might also useful to patch base to avoid some ifdefs)
With those patches, available in @kit-ty-kate 's opam-alpha-repository, both opam-health-check and opamcheck confirms that every packages compatible with 4.09 is compatible with 4.10+this PR.
For people (cc @gadmm) concerned about compatibility beyond 4.04, unfortunately, it will require some
#ifdefonOCAML_VERSION. However, Debian stable is at 4.05; and only oldstable has an OCaml version inferior to 4.04 . Taking in account that this is an internal API with no stability garantee, it seems fine to only provide a nice path for the 4.04 - 4.10 range.