Skip to content

Commit

Permalink
Fix LDouble.to_int
Browse files Browse the repository at this point in the history
`long double` must be casted to `intnat` first, not to `uintnat`
directly. Otherwise the conversion is undefined behavior for nearly
all negative values, see:
http://c0x.coding-guidelines.com/6.3.1.4.html , Note 50. The code was
correct until
ocaml/ocaml@24c118d
  • Loading branch information
fdopen committed Dec 6, 2018
1 parent 066cf8f commit bc5624a
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/ctypes/ldouble_stubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,9 @@ CAMLprim value ctypes_ldouble_of_int(value a) {
}
CAMLprim value ctypes_ldouble_to_int(value a) {
CAMLparam1(a);
CAMLreturn(Val_long(ldouble_custom_val(a)));
long double b = ldouble_custom_val(a);
intnat c = b;
CAMLreturn(Val_long(c));
}

#define OP2(OPNAME, OP) \
Expand Down
12 changes: 12 additions & 0 deletions tests/test-ldouble/test_ldouble.ml
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,18 @@ let test_conv _ =
List.iter (fun a -> assert_bool "to/of_float" (a = LDouble.(to_float (of_float a)))) flts;
assert_bool "to_int" (3 = LDouble.(to_int (of_float 3.45)));
assert_bool "to_int" (-34 = LDouble.(to_int (of_float (-34.999))));
if Sys.word_size = 32 then (
let mi = float_of_int min_int in
let ma = float_of_int max_int in
assert_bool "to_int" (min_int = LDouble.(to_int (of_float mi)));
assert_bool "to_int" (max_int = LDouble.(to_int (of_float ma)));
)
else (
let mi = -9007199254740991. in
let ma = 9007199254740991. in
assert_bool "to_int" (Int64.to_int (-9007199254740991L) = LDouble.(to_int (of_float mi)));
assert_bool "to_int" (Int64.to_int 9007199254740991L = LDouble.(to_int (of_float ma)));
);
assert_bool "of_string" (3.5 = LDouble.(to_float (of_string "3.5")));
assert_bool "to_string" ("3.500000" = LDouble.(to_string (of_float 3.5)))

Expand Down

0 comments on commit bc5624a

Please sign in to comment.