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
Expose OCaml version in C headers #6406
Comments
Comment author: @gasche The documentation/specification
Which "valid value" is used is not specified in the documentation, so I don't think it was correct to rely on a precise choice of initialization value in the first place. Could you explain the ctypes use-case? I had a glimpse of the discussion but that was quite cryptic to someone not familiar with the implementation. |
Comment author: @whitequark You do have a point. Still, the rest of my argument stands. The idea is that an OCaml callback fills some parts of an Obj.t array, and leaves some intact. The C code would then detect filled-in parts. |
Comment author: @mshinwell I agree with Gabriel. I'm not sure it's a good idea to rely on which particular initializer is used. Exposing a version string seems sensible. |
Comment author: @whitequark A string is nearly impossible to perform comparisons on; how about OCAML_MAJOR OCAML_MINOR OCAML_PATCH, and in addition OCAML_VERSION that has the previous concatenated. That's how I usually seen it done. E.g. for 3.12: OCAML_MAJOR = 3 For 4.01.00: OCAML_MAJOR = 4 This way you have both conveniently separated version parts and a way to compare them... e.g. #if OCAML_VERSION >= 40200L |
Comment author: @damiendoligez A good idea, but isn't it overkill to make it a long int? I doubt we'll ever reach OCaml version 400000.00.0 |
Comment author: @whitequark Hm, I think I cargo-culted the L suffix from the C-internal definitions, where you have conditions like "#if STDC_VERSION > 199900L". Not sure whether it's actually needed or it's some obscure leftover. |
Comment author: dinosaure I try to complete this issue with: #72 |
Comment author: @damiendoligez I want to change a few things before I apply this patch or the github one. Do you have a strong preference for the format of the OCAML_VERSION integer (i.e. decimal as here or binary as in the github patch)? I'd rather use decimal. |
Comment author: @whitequark Definitely decimal. Versions are in decimal, after all, it's just a form of fixed point. Also, I've never ever seen version constants in hex. |
Comment author: dinosaure It is done at your convenience. :) |
Comment author: @damiendoligez Patch modified and applied in 4.02 (commit 15016) and trunk (commit 15017). |
Original bug ID: 6406
Reporter: @whitequark
Assigned to: @damiendoligez
Status: closed (set by @damiendoligez on 2014-07-21T20:40:05Z)
Resolution: fixed
Priority: normal
Severity: minor
Target version: 4.02.0+dev
Fixed in version: 4.02.0+dev
Category: runtime system and C interface
Tags: patch, junior_job
Monitored by: @gasche
Bug description
I was specifically bitten by this commit: 05100e5#diff-f940572c32e55a7e40e7f693deea4c7bR42
Which changed, as far as I understand, a documented public API in an incompatible way between minor versions. This is horrible on its own, but the fact that there is no way to detect OCaml version in the C code to disambiguate makes it also impossible to work around.
File attachments
The text was updated successfully, but these errors were encountered: