Skip to content

Commit a06f36f

Browse files
bobzhangbobzhang
committed
fix a hashtbl hash bug, add more tests for hashtbl (#273)
1 parent 894b132 commit a06f36f

File tree

12 files changed

+320
-30
lines changed

12 files changed

+320
-30
lines changed

‎jscomp/lib/js.ml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ external anything_to_string : 'a -> string = "js_anything_to_string"
3333
external anything_to_number : 'a -> float = "js_anything_to_number"
3434

3535

36+
type any = Obj.t
37+
38+
external erase : 'a -> any = "%identity"
39+
external cast : any -> 'a = "%identity"
3640

3741
type + 'a opt
3842

@@ -141,10 +145,13 @@ module Caml_obj = struct
141145
external set_tag : Obj.t -> int -> unit = "caml_obj_set_tag"
142146
external uninitialized_object : int -> int -> Obj.t = "js_uninitialized_object"
143147
external is_instance_array : Obj.t -> bool = "js_is_instance_array" (* use Array.isArray instead*)
148+
external size_of_any : Obj.t -> 'a def = "length" [@@bs.get]
149+
external tag_of_any : Obj.t -> 'a def = "tag" [@@bs.get]
144150
end
145151

146152
module Caml_int64 = struct
147153
external discard_sign : int64 -> int64 = "js_int64_discard_sign"
148154
external div_mod : int64 -> int64 -> int64 * int64 = "js_int64_div_mod"
149155
external to_hex : int64 -> string = "js_int64_to_hex"
150156
end
157+

‎jscomp/runtime/.depend‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ caml_queue.cmi :
1515
caml_string.cmi :
1616
caml_sys.cmi :
1717
caml_utils.cmi :
18-
caml_weak.cmi :
1918
caml_array.cmo : js.cmo caml_array.cmi
2019
caml_array.cmx : js.cmx caml_array.cmi
2120
caml_bigarray.cmo : caml_bigarray.cmi
@@ -54,8 +53,6 @@ caml_sys.cmo : js.cmo caml_sys.cmi
5453
caml_sys.cmx : js.cmx caml_sys.cmi
5554
caml_utils.cmo : caml_utils.cmi
5655
caml_utils.cmx : caml_utils.cmi
57-
caml_weak.cmo : caml_array.cmi caml_weak.cmi
58-
caml_weak.cmx : caml_array.cmx caml_weak.cmi
5956
fn.cmo :
6057
fn.cmx :
6158
js.cmo :
@@ -102,8 +99,6 @@ caml_sys.cmo : js.cmo caml_sys.cmi
10299
caml_sys.cmj : js.cmj caml_sys.cmi
103100
caml_utils.cmo : caml_utils.cmi
104101
caml_utils.cmj : caml_utils.cmi
105-
caml_weak.cmo : caml_array.cmi caml_weak.cmi
106-
caml_weak.cmj : caml_array.cmj caml_weak.cmi
107102
fn.cmo :
108103
fn.cmj :
109104
js.cmo :

‎jscomp/runtime/caml_hash.js‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ function caml_hash(count, _, seed, obj) {
6969
while(queue[/* length */0] !== 0 && num > 0) {
7070
var obj$1 = Caml_queue.unsafe_pop(queue);
7171
if (typeof obj$1 === "number") {
72-
hash = mix(hash, obj$1 | 0);
72+
var u$1 = obj$1 | 0;
73+
hash = mix(hash, (u$1 + u$1 | 0) + 1 | 0);
7374
num = num - 1 | 0;
7475
}
7576
else if (typeof obj$1 === "string") {
@@ -83,19 +84,29 @@ function caml_hash(count, _, seed, obj) {
8384
Caml_builtin_exceptions.assert_failure,
8485
[
8586
"caml_hash.ml",
86-
124,
87+
125,
8788
8
8889
]
8990
];
9091
}
9192
else if (typeof obj$1 !== "function") {
92-
var tag = obj$1.tag | 0;
93-
hash = mix(hash, tag);
94-
var v = obj$1.length - 1 | 0;
95-
var block = v < num ? v : num;
96-
for(var i = 0; i<= block; ++i){
97-
Caml_queue.push(obj$1[i], queue);
93+
var size = obj$1.length;
94+
if (size) {
95+
var obj_tag = obj$1.tag | 0;
96+
var tag = (size << 10) | obj_tag;
97+
if (tag === 248) {
98+
hash = mix(hash, obj$1[1]);
99+
}
100+
else {
101+
hash = mix(hash, tag);
102+
var v = size - 1 | 0;
103+
var block = v < num ? v : num;
104+
for(var i = 0; i<= block; ++i){
105+
Caml_queue.push(obj$1[i], queue);
106+
}
107+
}
98108
}
109+
99110
}
100111

101112
}

‎jscomp/runtime/caml_hash.ml‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ let caml_hash count _limit seed obj =
108108
let obj = Caml_queue.unsafe_pop queue in
109109
if Js.typeof obj = "number" then
110110
begin
111-
hash := mix !hash (Nativeint.of_float (Obj.magic obj));
111+
let u = Nativeint.of_float (Obj.magic obj) in
112+
hash := mix !hash (u +~ u +~ 1n) ;
112113
decr num ;
113114
end
114115
else if Js.typeof obj = "string" then
@@ -125,13 +126,23 @@ let caml_hash count _limit seed obj =
125126
else if Js.typeof obj = "function" then
126127
()
127128
else
128-
let tag = Obj.tag obj in
129-
hash := mix !hash (Nativeint.of_int tag) ;
130-
let block =
131-
let v = Obj.size obj - 1 in if v < !num then v else !num in
132-
for i = 0 to block do
133-
Caml_queue.push (Obj.field obj i ) queue
134-
done
129+
let size = Js.Caml_obj.size_of_any obj in
130+
match Js.from_def size with (* FIXME: generated code is buggy *)
131+
| None -> ()
132+
| Some size ->
133+
let obj_tag = Obj.tag obj in
134+
let tag = (size lsl 10) lor obj_tag in
135+
if tag = 248 (* Obj.object_tag*) then
136+
hash := mix !hash (Nativeint.of_int (Oo.id (Obj.magic obj)))
137+
else
138+
begin
139+
hash := mix !hash (Nativeint.of_int tag) ;
140+
let block =
141+
let v = size - 1 in if v < !num then v else !num in
142+
for i = 0 to block do
143+
Caml_queue.push (Obj.field obj i ) queue
144+
done
145+
end
135146
done;
136147
final_mix !hash
137148

‎jscomp/test/.depend‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,10 @@ int64_test.cmo : ../stdlib/pervasives.cmi ../stdlib/nativeint.cmi mt.cmi \
218218
int64_test.cmx : ../stdlib/pervasives.cmx ../stdlib/nativeint.cmx mt.cmx \
219219
../lib/js.cmx ../stdlib/int64.cmx ../stdlib/int32.cmx \
220220
../stdlib/format.cmx ext_array.cmx ../stdlib/array.cmx
221+
int_hashtbl_test.cmo : mt.cmi ../stdlib/list.cmi ../stdlib/hashtbl.cmi \
222+
../stdlib/array.cmi
223+
int_hashtbl_test.cmx : mt.cmx ../stdlib/list.cmx ../stdlib/hashtbl.cmx \
224+
../stdlib/array.cmx
221225
int_map.cmo : ../stdlib/map.cmi
222226
int_map.cmx : ../stdlib/map.cmx
223227
int_overflow_test.cmo : ../stdlib/string.cmi mt.cmi ../stdlib/int32.cmi \
@@ -764,6 +768,10 @@ int64_test.cmo : ../stdlib/pervasives.cmi ../stdlib/nativeint.cmi mt.cmi \
764768
int64_test.cmj : ../stdlib/pervasives.cmj ../stdlib/nativeint.cmj mt.cmj \
765769
../lib/js.cmj ../stdlib/int64.cmj ../stdlib/int32.cmj \
766770
../stdlib/format.cmj ext_array.cmj ../stdlib/array.cmj
771+
int_hashtbl_test.cmo : mt.cmi ../stdlib/list.cmi ../stdlib/hashtbl.cmi \
772+
../stdlib/array.cmi
773+
int_hashtbl_test.cmj : mt.cmj ../stdlib/list.cmj ../stdlib/hashtbl.cmj \
774+
../stdlib/array.cmj
767775
int_map.cmo : ../stdlib/map.cmi
768776
int_map.cmj : ../stdlib/map.cmj
769777
int_overflow_test.cmo : ../stdlib/string.cmi mt.cmi ../stdlib/int32.cmi \

‎jscomp/test/hash_test.js‎

Lines changed: 71 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,23 +63,86 @@ function normalize(x) {
6363
return x & 1073741823;
6464
}
6565

66-
var param = $$Array.map(function (x) {
67-
return Hashtbl.hash(x) & 1073741823;
68-
}, test_strings);
66+
function caml_hash(x) {
67+
return Hashtbl.hash(x) & 1073741823;
68+
}
69+
70+
var param = $$Array.map(caml_hash, test_strings);
6971

70-
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 17, characters 5-12', param, test_strings_hash_results);
72+
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 18, characters 5-12', param, test_strings_hash_results);
7173

7274
var param$1 = Hashtbl.hash(0) & 1073741823;
7375

74-
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 23, characters 5-12', param$1, 129913994);
76+
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 24, characters 5-12', param$1, 129913994);
7577

7678
var param$2 = Hashtbl.hash("x") & 1073741823;
7779

78-
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 26, characters 5-12', param$2, 780510073);
80+
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 27, characters 5-12', param$2, 780510073);
7981

8082
var param$3 = Hashtbl.hash("xy") & 1073741823;
8183

82-
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 29, characters 5-12', param$3, 194127723);
84+
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 30, characters 5-12', param$3, 194127723);
85+
86+
var param$4 = Hashtbl.hash(/* A */65) & 1073741823;
87+
88+
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 33, characters 5-12', param$4, 381663642);
89+
90+
var param$5 = Hashtbl.hash(/* `A */[
91+
65,
92+
3
93+
]) & 1073741823;
94+
95+
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 34, characters 5-12', param$5, 294279345);
96+
97+
var param$6 = Hashtbl.hash(/* :: */[
98+
/* `A */[
99+
65,
100+
3
101+
],
102+
/* :: */[
103+
/* `B */[
104+
66,
105+
2
106+
],
107+
/* :: */[
108+
/* `C */[
109+
67,
110+
3
111+
],
112+
/* [] */0
113+
]
114+
]
115+
]) & 1073741823;
116+
117+
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 35, characters 5-12', param$6, 1017654909);
118+
119+
var param$7 = Hashtbl.hash(/* :: */[
120+
/* tuple */[
121+
/* `A */[
122+
65,
123+
"3"
124+
],
125+
/* `B */[
126+
66,
127+
"2"
128+
]
129+
],
130+
/* :: */[
131+
/* tuple */[
132+
/* `C */[
133+
67,
134+
"3"
135+
],
136+
/* `D */[
137+
68,
138+
"4"
139+
]
140+
],
141+
/* [] */0
142+
]
143+
]) & 1073741823;
144+
145+
Mt_global.collect_eq(test_id, suites, 'File "hash_test.ml", line 36, characters 5-12', param$7, 81986873);
83146

84147
Mt.from_pair_suites("hash_test.ml", suites[0]);
85148

@@ -89,4 +152,5 @@ exports.eq = eq;
89152
exports.test_strings = test_strings;
90153
exports.test_strings_hash_results = test_strings_hash_results;
91154
exports.normalize = normalize;
155+
exports.caml_hash = caml_hash;
92156
/* test_strings Not a pure module */

‎jscomp/test/hash_test.ml‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,10 @@ let test_strings_hash_results =
1313
178511779; 585018975; 544388424; 1043872806; 831138595|]
1414

1515
let normalize x = x land 0x3FFFFFFF
16+
let caml_hash x = Hashtbl.hash x |> normalize
1617
let () =
1718
eq __LOC__
18-
(test_strings |> Array.map (fun x -> normalize (Hashtbl.hash x) ))
19+
(test_strings |> Array.map caml_hash)
1920
test_strings_hash_results
2021

2122

@@ -28,5 +29,12 @@ let () =
2829
let () =
2930
eq __LOC__ (Hashtbl.hash "xy" |> normalize) 194127723
3031

31-
let () =
32+
let () =
33+
eq __LOC__ (caml_hash `A) 381663642;
34+
eq __LOC__ (caml_hash (`A 3)) 294279345;
35+
eq __LOC__ (caml_hash [`A 3; `B 2 ; `C 3 ]) 1017654909;
36+
eq __LOC__ (caml_hash [`A "3", `B "2" ; `C "3", `D "4"]) (81986873)
37+
38+
39+
let () =
3240
Mt.from_pair_suites __FILE__ !suites

0 commit comments

Comments
 (0)