Update Tenv.ml

[Jatkingmodern]

Please see CONTRIBUTING.md for how to set up your development environment and run tests.

[davidpichardie]

The impact of this optimisation is questionnable.

• Now we have 2 accesses instead of one
• It is rare clients will run twice mk_struct on the same key

[Jatkingmodern]

TypenameHash.replace tenv name struct_typ

Single store operation, minimal churn in code and contention.

If duplicates are rare, extra writes are negligible compared to the cost of the lookup logic.

If you really want to avoid unnecessary writes, do a single find + conditional replace

match TypenameHash.find_opt tenv name with
| Some prev when prev == struct_typ -> ()
| _ -> TypenameHash.replace tenv name struct_typ

This avoids a useless replace when the stored value is physically identical to struct_typ.

It still performs one lookup in the common (no-change) case and a replace only when necessary. Note this is still one lookup + occasional replace, not cheaper than always replacing in terms of number of operations — but it avoids churn when phys_equal holds.

If structural equality rather than physical equality is required, the check becomes more expensive (deep compare) and probably not worth it.

[davidpichardie]

It seems just equivalent to the previous version.

[davidpichardie]

I prefer the terminal recursive function instead of a while loop. Matter of taste ;-)

[Jatkingmodern]

(* before: (pseudo)
let worklist = ref [name] in
let visited = ref Typ.Name.Set.empty in
let result = ref init in
while !worklist <> [] do
let x = List.hd !worklist in
worklist := List.tl !worklist;
if Typ.Name.Set.mem x !visited then ()
else begin
visited := Typ.Name.Set.add x !visited;
(* update result and push new items into worklist *)
end
done;
!result
*)

(* After: tail-recursive )
let rec loop (worklist : Typ.Name.t list) (visited : Typ.Name.Set.t) (result : 'a) : 'a =
match worklist with
| [] -> result
| x :: rest ->
if Typ.Name.Set.mem x visited then
loop rest visited result
else
let visited' = Typ.Name.Set.add x visited in
( compute result' and new_items for the worklist; adapt these lines )
let (result', new_items) = ( ... your processing of x ... *) in
loop (new_items @ rest) visited' result'

let run name init =
loop [name] Typ.Name.Set.empty init

[davidpichardie]

what is you objective here?

[Jatkingmodern]

Readable exception-based version (recommended if fold_supers cannot early-return)
(* Search supers and return the first Some result from f )
let find_in_supers ?ignore_require_extends tenv name ~f =
( local exception used to short-circuit the fold when a result is found *)
exception Found of 'a
in
try
fold_supers ?ignore_require_extends tenv name ~init:() ~f:(fun name struct_opt () ->
match f name struct_opt with
| None -> ()
| Some _ as res -> raise (Found res)
);
None
with
| Found res -> res

[davidpichardie]

!!!!!!!!!?????

[Jatkingmodern]

let find_in_supers ?ignore_require_extends tenv name ~f =
let result = ref None in
let _ =
fold_supers ?ignore_require_extends tenv name ~init:() ~f:(fun name struct_opt () ->
match !result with
| Some _ -> () (* already found; skip further processing *)
| None ->
(match f name struct_opt with
| None -> ()
| Some r -> result := Some r)
)
in
!result