From ef7650ec99639a403a5b4c3c30fda86f405b7737 Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Sat, 12 Sep 2026 16:02:16 +0700 Subject: [PATCH] A kebab collision takes every name in its group down MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two C functions whose names kebab to one Flan name used to resolve by order: the first won the name, the second was refused. Which one that is depends on the order the header happens to declare them in, so moving two lines in somebody else's header would silently rebind a name a Flan program is already calling — and the winner was left in the hidden list too, so using the name it did get reported that it could not be had. Neither takes it now. There is no reading of spin-2d that is obviously right when the header offers both Spin2D and spin2d, so both are refused and both say why; the author binds the one they want with a hand-written declare-c, which is what that form is for. Found by test/headers/sample.h, which is why it is a fixture rather than a raylib case. raylib is unaffected: its 581 names are injective under the rule. --- lib/cimport.ml | 63 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 48 insertions(+), 15 deletions(-) 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 ───────────── *)