Mantis Bug Tracker

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0006406OCamlOCaml runtime systempublic2014-05-08 20:142014-07-21 22:40
Reporterwhitequark 
Assigned Todoligez 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusclosedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version4.02.0+devFixed in Version4.02.0+dev 
Summary0006406: Expose OCaml version in C headers
DescriptionI was specifically bitten by this commit: https://github.com/ocaml/ocaml/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.
Tagsjunior_job, patch
Attached Files? file icon patch.6406 [^] (2,193 bytes) 2014-06-19 21:41

- Relationships

-  Notes
(0011410)
gasche (developer)
2014-05-08 23:13

The documentation/specification
  http://caml.inria.fr/pub/docs/manual-ocaml/intfc.html [^]
says:

> caml_alloc(n, t) returns a fresh block of size n with tag t.
> If t is less than No_scan_tag, then the fields of the block
> are initialized with a valid value in order to satisfy
> the GC constraints.

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.
(0011411)
whitequark (developer)
2014-05-08 23:21

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.
(0011414)
shinwell (developer)
2014-05-09 12:48

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.
(0011415)
whitequark (developer)
2014-05-09 13:06

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
  OCAML_MINOR = 12
  OCAML_PATCH = 00
  OCAML_VERSION = 31200L

For 4.01.00:

  OCAML_MAJOR = 4
  OCAML_MINOR = 01
  OCAML_PATCH = 00
  OCAML_VERSION = 40100L

This way you have both conveniently separated version parts and a way to compare them... e.g. #if OCAML_VERSION >= 40200L
(0011499)
doligez (administrator)
2014-05-16 18:02

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
(0011500)
whitequark (developer)
2014-05-16 18:04

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.
(0011760)
dinosaure (reporter)
2014-06-19 21:35

I try to complete this issue with: https://github.com/ocaml/ocaml/pull/72 [^]
(0011856)
doligez (administrator)
2014-07-16 17:13

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.
(0011857)
whitequark (developer)
2014-07-16 17:18

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.
(0011877)
dinosaure (reporter)
2014-07-17 13:32

It is done at your convenience. :)
(0011895)
doligez (administrator)
2014-07-21 22:40

Patch modified and applied in 4.02 (commit 15016) and trunk (commit 15017).

- Issue History
Date Modified Username Field Change
2014-05-08 20:14 whitequark New Issue
2014-05-08 23:13 gasche Note Added: 0011410
2014-05-08 23:21 whitequark Note Added: 0011411
2014-05-09 12:48 shinwell Note Added: 0011414
2014-05-09 13:06 whitequark Note Added: 0011415
2014-05-09 14:42 gasche Status new => acknowledged
2014-05-16 18:02 doligez Note Added: 0011499
2014-05-16 18:04 whitequark Note Added: 0011500
2014-06-19 17:56 gasche Tag Attached: junior_job
2014-06-19 21:35 dinosaure Note Added: 0011760
2014-06-19 21:41 dinosaure File Added: patch.6406
2014-07-16 17:06 doligez Tag Attached: patch
2014-07-16 17:13 doligez Note Added: 0011856
2014-07-16 17:13 doligez Status acknowledged => feedback
2014-07-16 17:13 doligez Target Version => 4.02.0+dev
2014-07-16 17:18 whitequark Note Added: 0011857
2014-07-16 17:18 whitequark Status feedback => new
2014-07-17 12:27 shinwell Assigned To => doligez
2014-07-17 12:27 shinwell Status new => assigned
2014-07-17 13:32 dinosaure Note Added: 0011877
2014-07-21 22:40 doligez Note Added: 0011895
2014-07-21 22:40 doligez Status assigned => closed
2014-07-21 22:40 doligez Resolution open => fixed
2014-07-21 22:40 doligez Fixed in Version => 4.02.0+dev


Copyright © 2000 - 2011 MantisBT Group
Powered by Mantis Bugtracker