Skip to content

Commit 244b531

Browse files
committed
Fix a bug in subst noticed in uwt
Two fixes and a test: 1. Slightly more recursion needed when expanding `module type of` expressions before compilation 2. In the presence of `module type of` expressions and `includes`, we can get in the situation when we try to rename elements that have been prefixed. This is not an error, though it is unusual. we end up labelling the items with different idents, but that's fine because all expressions referencing them are using the prefixed path anyway. The test is a much cut-down version of `uwt` that was originally demonstrating the problem, manifesting as an internal `Failure` begin raised
1 parent fcedc4e commit 244b531

File tree

4 files changed

+92
-5
lines changed

4 files changed

+92
-5
lines changed

src/xref2/subst.ml

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -905,20 +905,26 @@ and rename_bound_idents s sg =
905905
try
906906
match PathModuleMap.find (id :> Ident.path_module) s.module_ with
907907
| `Renamed (`LModule _ as x) -> x
908+
| `Prefixed (_, _) ->
909+
(* This is unusual but can happen when we have TypeOf expressions. It means
910+
we're already prefixing this module path, hence we can essentially rename
911+
it to whatever we like because it's never going to be referred to. *)
912+
Ident.Rename.module_ id
908913
| _ -> failwith "Error"
909914
with Not_found -> Ident.Rename.module_ id
910915
in
911916
let new_module_type_id id =
912917
try
913918
match ModuleTypeMap.find id s.module_type with
914919
| `Renamed x -> x
915-
| _ -> failwith "Error"
920+
| `Prefixed (_, _) -> Ident.Rename.module_type id
916921
with Not_found -> Ident.Rename.module_type id
917922
in
918923
let new_type_id id =
919924
try
920925
match PathTypeMap.find (id :> Ident.path_type) s.type_ with
921926
| `Renamed (`LType _ as x) -> x
927+
| `Prefixed (_, _) -> Ident.Rename.type_ id
922928
| _ -> failwith "Error"
923929
with Not_found -> Ident.Rename.type_ id
924930
in
@@ -928,6 +934,7 @@ and rename_bound_idents s sg =
928934
PathClassTypeMap.find (id :> Ident.path_class_type) s.class_type
929935
with
930936
| `Renamed (`LClass _ as x) -> x
937+
| `Prefixed (_, _) -> Ident.Rename.class_ id
931938
| _ -> failwith "Error"
932939
with Not_found -> Ident.Rename.class_ id
933940
in
@@ -937,6 +944,7 @@ and rename_bound_idents s sg =
937944
PathClassTypeMap.find (id :> Ident.path_class_type) s.class_type
938945
with
939946
| `Renamed (`LClassType _ as x) -> x
947+
| `Prefixed (_, _) -> Ident.Rename.class_type id
940948
| _ -> failwith "Error!"
941949
with Not_found -> Ident.Rename.class_type id
942950
in

src/xref2/type_of.ml

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ and module_type_expr env (id : Id.Signature.t) expr =
6969
| TypeOf t -> (
7070
match module_type_expr_typeof env id t with
7171
| Ok e ->
72-
TypeOf
73-
{ t with t_expansion = Some Lang_of.(simple_expansion empty id e) }
72+
let se = Lang_of.(simple_expansion empty id e) in
73+
TypeOf { t with t_expansion = Some (simple_expansion env se) }
7474
| Error e
7575
when Errors.is_unexpanded_module_type_of (e :> Errors.Tools_error.any)
7676
->
@@ -86,8 +86,8 @@ and u_module_type_expr env id expr =
8686
| TypeOf t -> (
8787
match module_type_expr_typeof env id t with
8888
| Ok e ->
89-
TypeOf
90-
{ t with t_expansion = Some Lang_of.(simple_expansion empty id e) }
89+
let se = Lang_of.(simple_expansion empty id e) in
90+
TypeOf { t with t_expansion = Some (simple_expansion env se) }
9191
| Error e
9292
when Errors.is_unexpanded_module_type_of (e :> Errors.Tools_error.any)
9393
->
@@ -98,6 +98,14 @@ and u_module_type_expr env id expr =
9898
and functor_parameter env p =
9999
{ p with expr = module_type_expr env (p.id :> Id.Signature.t) p.expr }
100100

101+
and simple_expansion :
102+
Env.t -> ModuleType.simple_expansion -> ModuleType.simple_expansion =
103+
fun env -> function
104+
| Signature sg -> Signature (signature env sg)
105+
| Functor (Named n, sg) ->
106+
Functor (Named (functor_parameter env n), simple_expansion env sg)
107+
| Functor (Unit, sg) -> Functor (Unit, simple_expansion env sg)
108+
101109
and include_ env i =
102110
let decl =
103111
match i.decl with
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
Repro of problem from uwt (https://github.com/ocaml/odoc/issues/691)
2+
3+
$ cat uwt_base.mli
4+
(* This file is part of uwt, released under the MIT license. See LICENSE.md for
5+
details, or visit https://github.com/fdopen/uwt/blob/master/LICENSE.md. *)
6+
module Base : sig
7+
type 'a uv_result = 'a
8+
9+
module Fs_types : sig
10+
type uv_open_flag =
11+
| O_RDONLY (** Open for reading *)
12+
(** Flags for {!Fs_functions.openfile}
13+
14+
[O_CLOEXEC] doesn't exist, because this flag is unconditionally
15+
added by libuv. [O_SHARE_DELETE], [O_SHARE_WRITE], [O_SHARE_READ]
16+
are always added on Windows, unless [O_EXLOCK] is specified. *)
17+
18+
end
19+
20+
module type Fs_functions = sig
21+
include module type of Fs_types
22+
with type uv_open_flag = Fs_types.uv_open_flag
23+
24+
type 'a t
25+
26+
val openfile : ?perm:int -> mode:uv_open_flag list -> string -> int t
27+
(** Equivalent to open(2). perm defaults are 0o644 *)
28+
end
29+
end
30+
31+
include module type of Base
32+
with type Fs_types.uv_open_flag = Base.Fs_types.uv_open_flag
33+
34+
35+
What used to happen is that the `odoc link` command would cause an internal
36+
error. If it doesn't here, that particular issue is fixed!
37+
38+
$ ocamlc -c -bin-annot uwt_base.mli
39+
$ odoc compile uwt_base.cmti
40+
$ odoc link uwt_base.odoc
41+
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
(* This file is part of uwt, released under the MIT license. See LICENSE.md for
2+
details, or visit https://github.com/fdopen/uwt/blob/master/LICENSE.md. *)
3+
module Base : sig
4+
type 'a uv_result = 'a
5+
6+
module Fs_types : sig
7+
type uv_open_flag =
8+
| O_RDONLY (** Open for reading *)
9+
(** Flags for {!Fs_functions.openfile}
10+
11+
[O_CLOEXEC] doesn't exist, because this flag is unconditionally
12+
added by libuv. [O_SHARE_DELETE], [O_SHARE_WRITE], [O_SHARE_READ]
13+
are always added on Windows, unless [O_EXLOCK] is specified. *)
14+
15+
end
16+
17+
module type Fs_functions = sig
18+
include module type of Fs_types
19+
with type uv_open_flag = Fs_types.uv_open_flag
20+
21+
type 'a t
22+
23+
val openfile : ?perm:int -> mode:uv_open_flag list -> string -> int t
24+
(** Equivalent to open(2). perm defaults are 0o644 *)
25+
end
26+
end
27+
28+
include module type of Base
29+
with type Fs_types.uv_open_flag = Base.Fs_types.uv_open_flag
30+

0 commit comments

Comments
 (0)