diff --git a/lib/cimport.ml b/lib/cimport.ml index 058b77a..cb5d2f8 100644 --- a/lib/cimport.ml +++ b/lib/cimport.ml @@ -511,26 +511,50 @@ type imported = { [foo] whose signature has a struct in it, and a collision there is refused for the whole program rather than for the one binding. *) let of_dump ~env ~taken ~bound_syms (d : dump) : imported = - let decls = ref [] and hidden = ref [] and by_name = Hashtbl.create 512 in + let decls = ref [] and hidden = ref [] in + (* Collisions are found before anything is emitted, and they take *every* + name in the colliding group down with them. + + Resolving one by taking the first and refusing the rest is the tempting + shape and the wrong one: which C function ends up owning the Flan name + would then depend on the order the header happens to declare them in, so + moving two lines in somebody else's header silently rebinds a name a Flan + program is already calling. There is no reading of [spin-2d] that is + obviously right when the header offers both [Spin2D] and [spin2d], so + neither gets it, and both say why. The author disambiguates with a + hand-written declare-c, which is what that form is for. + + [bound_syms] is excluded first: a C function the package already binds by + hand is not competing for an imported name at all, so it cannot collide + with one. *) + let candidates = + List.filter (fun f -> not (List.mem f.csym bound_syms)) d.fns + in + let groups = Hashtbl.create 512 in + List.iter + (fun f -> + let k = kebab f.csym in + Hashtbl.replace groups k (f.csym :: Option.value ~default:[] + (Hashtbl.find_opt groups k))) + candidates; List.iter (fun f -> let flan = kebab f.csym in let skip why = hidden := (flan, why) :: !hidden in - if List.mem f.csym bound_syms then - (* Not hidden: the name the package wrote for it is there and works. - This is the escape hatch doing its job. *) - () - else if Hashtbl.mem taken flan || Hashtbl.mem taken (flan ^ "-c") then + match List.rev (Hashtbl.find groups flan) with + | _ :: _ :: _ as all -> + skip + (Printf.sprintf + "%s all kebab to %s, and which one got the name would depend on \ + the order the header declares them in — so none of them takes \ + it. Bind the one you want with a hand-written declare-c" + (String.concat ", " all) flan) + | _ -> + if Hashtbl.mem taken flan || Hashtbl.mem taken (flan ^ "-c") then skip (Printf.sprintf "%s would be the imported name of %s, and the package declares \ %s already" flan f.csym flan) - else if Hashtbl.mem by_name flan then - skip - (Printf.sprintf - "%s and %s both kebab to %s, so the header cannot be imported \ - whole — one of them needs a hand-written declare-c" - (Hashtbl.find by_name flan) f.csym flan) else if f.cvariadic then skip (Printf.sprintf @@ -555,7 +579,6 @@ let of_dump ~env ~taken ~bound_syms (d : dump) : imported = with | Error why -> skip (Printf.sprintf "%s %s" f.csym why) | Ok (params, ret) -> - Hashtbl.replace by_name flan f.csym; decls := { Ast.d = Ast.DeclareC @@ -563,8 +586,18 @@ let of_dump ~env ~taken ~bound_syms (d : dump) : imported = f.csym); dloc = f.cloc } :: !decls) - d.fns; - { decls = List.rev !decls; hidden = List.rev !hidden } + candidates; + (* One entry per name. A collision refuses every member of its group and each + of them writes the same reason under the same name, which [refuse_hidden] + would look up identically but a report would print twice. *) + let seen = Hashtbl.create 64 in + let hidden = + List.filter + (fun (n, _) -> + if Hashtbl.mem seen n then false else (Hashtbl.add seen n (); true)) + (List.rev !hidden) + in + { decls = List.rev !decls; hidden } (* ── Checking the package's layouts against the header's ───────────── *)