Skip to content

Commit 139986d

Browse files
cknittclaude
andcommitted
Key existential constructors by identifier
Codex review on #9080: the constructor of each existential was recorded by its name, but the counter that makes names unique is reset for each binding, so a nested module's `$a` could overwrite the constructor of an outer `$a`. It is recorded by the identifier's stamp now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kbf3gLMawVgtmiKy3TMBc3 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
1 parent 30a1a6a commit 139986d

5 files changed

Lines changed: 42 additions & 11 deletions

File tree

‎compiler/ml/ctype.ml‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -932,12 +932,15 @@ let new_declaration newtype manifest =
932932
type_inlined_types = [];
933933
}
934934

935-
(* The constructor binding each existential type named by
936-
[instance_constructor], for explaining an escaping one. *)
935+
(* The constructor binding each existential type created by
936+
[instance_constructor], by the stamp of its identifier, for explaining an
937+
escaping one. Names are not unique: their counter is reset per binding. *)
937938
let existential_constructors_key =
938939
Request_local.new_key (fun () -> Hashtbl.create 8)
939-
let existential_constructor name =
940-
Hashtbl.find_opt (Domain.DLS.get existential_constructors_key) name
940+
let existential_constructor id =
941+
Hashtbl.find_opt
942+
(Domain.DLS.get existential_constructors_key)
943+
(Ident.binding_time id)
941944

942945
let instance_constructor ?in_pattern cstr =
943946
with_copy_session (fun () ->
@@ -953,11 +956,11 @@ let instance_constructor ?in_pattern cstr =
953956
| {desc = Tvar (Some name)} -> "$" ^ name
954957
| _ -> "$" ^ cstr.cstr_name
955958
in
956-
let name = get_new_abstract_name name in
959+
let id = Ident.create (get_new_abstract_name name) in
957960
Hashtbl.replace
958961
(Domain.DLS.get existential_constructors_key)
959-
name cstr.cstr_name;
960-
let path = Path.Pident (Ident.create name) in
962+
(Ident.binding_time id) cstr.cstr_name;
963+
let path = Path.Pident id in
961964
let new_env = Env.add_local_type path decl !env in
962965
env := new_env;
963966
let to_unify = newty (Tconstr (path, [], ref Mnil)) in

‎compiler/ml/ctype.mli‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,9 @@ val instance_constructor :
172172
type_expr list * type_expr
173173
(* Same, for a constructor *)
174174

175-
val existential_constructor : string -> string option
176-
(* The constructor that binds the existential type of this name, given to it
177-
by [instance_constructor] in the current request *)
175+
val existential_constructor : Ident.t -> string option
176+
(* The constructor that binds this existential type, created by
177+
[instance_constructor] in the current request *)
178178

179179
val instance_parameterized_type :
180180
?keep_names:bool -> type_expr list -> type_expr -> type_expr list * type_expr

‎compiler/ml/printtyp.ml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1303,7 +1303,7 @@ let rec mismatch = function
13031303
let escape_explanation ppf p =
13041304
match p with
13051305
| Pident id -> (
1306-
match Ctype.existential_constructor (Ident.name id) with
1306+
match Ctype.existential_constructor id with
13071307
| Some constructor ->
13081308
fprintf ppf
13091309
"@,\
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
2+
We've found a bug for you!
3+
/.../fixtures/gadt_existential_nested_binding.res:12:5
4+
5+
10 ┆ }
6+
11 ┆ }
7+
12 ┆ x
8+
13 ┆ | Second(_) => assert(false)
9+
14 ┆ }
10+
11+
This has type: $a
12+
But it's expected to have type: 'a
13+
$a is an existential type bound by the constructor First.
14+
It is only known in the scope of the pattern that matches First.
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
type rec any = First('a): any | Second('a): any
2+
3+
let get = (v, w) =>
4+
switch v {
5+
| First(x) =>
6+
module M = {
7+
let inner = switch w {
8+
| Second(y) => ignore(y)
9+
| First(_) => ()
10+
}
11+
}
12+
x
13+
| Second(_) => assert(false)
14+
}

0 commit comments

Comments
 (0)