stdlib: add Pervasives.int_size#1572
Conversation
581b18d to
71d0ff2
Compare
|
I am not convinced that this value needs to be in Pervasives. On the other hand, the changes to |
|
But would it be fine to refer to Sys from Pervasives? This seems like unnecessary semantic dependency. |
stdlib/pervasives.mli
Outdated
|
|
||
| val int_size : int | ||
| (** Size of an integer, in bits. It is 31 (resp. 63) when using the OCaml | ||
| compiler on a 32-bit (resp. 64-bit) platform. It may differ for other |
There was a problem hiding this comment.
Shouldn't we refer to the target machine/architecture rather than to compilers?
There was a problem hiding this comment.
How about this?
(** Size of an integer, in bits. It is 31 (resp. 63) when using OCaml on a
32-bit (resp. 64-bit) platform. It may differ for other implementations,
e.g. it can be 32 bits when compiling to JavaScript.
@since 4.07.0
*)
stdlib/pervasives.mli
Outdated
| where [bitsize] is [32] on a 32-bit platform and | ||
| [64] on a 64-bit platform. | ||
| Right-associative operator at precedence level 8/11. *) | ||
| The result is unspecified if [m < 0] or [m > int_size]. |
There was a problem hiding this comment.
Isn't [int_size] just an alias for [bit_size]?
(If so, the characterization of unspecified
results has changed)
There was a problem hiding this comment.
No, bit_size = 32 / 64 and int_size = 31 / 63
There was a problem hiding this comment.
Of course, sorry for the noise.
|
I agree with @gasche that it doesn't seem necessary to duplicate the symbol. I don't personally see a problem with documentation referring to modules against the direction of the dependency chain. |
71d0ff2 to
97aa362
Compare
|
Alright, I feel convinced that duplicating |
damiendoligez
left a comment
There was a problem hiding this comment.
This version looks good.
|
@murmour Can you rebase on trunk? |
97aa362 to
20f39bd
Compare
Done. |
Integer size is an important part of arithmetic specification, so it makes sense to have it in Pervasives, for the purpose of improving documentation.
Now, the docstrings for
lsl,lsr, andasrare compatible with Js_of_ocaml.