Skip to content

Commit 18392ab

Browse files
authored
flambda-backend: Backport 13099 (#2529)
* Add regression test (with exn-raising output) * Fix bug by backporting 13099 * push a small difference to upstream * apology
1 parent 889c081 commit 18392ab

File tree

10 files changed

+87
-2
lines changed

10 files changed

+87
-2
lines changed
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
type t = unit
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
let f (_ : Lib.t) = ()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
type t = bool
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
let f (_ : Lib.t) = ()
2+
3+
(* The naming of this module is important: When the error reporting
4+
is running in a mode where it can load new cmis from disk, this
5+
module leads the compiler to try to load a cmi file [lib1_client.cmi].
6+
That's because the compiler tries to be smart about double-underscore
7+
paths, rewriting [Foo__Bar] to [Foo.Bar] when these names are aliases.
8+
*)
9+
module Lib1_client__X = struct
10+
type t = A
11+
end
12+
13+
module F (T : sig type t end) = struct
14+
type t = Lib1_client__X.t
15+
16+
let f (_ : T.t) = ()
17+
end
18+
19+
module _ = F (struct type t = T end)
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
File "lib2_client.ml", line 19, characters 11-36:
2+
19 | module _ = F (struct type t = T end)
3+
^^^^^^^^^^^^^^^^^^^^^^^^^
4+
Error: This functor has type
5+
functor (T : sig type t end) ->
6+
sig type t = Lib1_client__X.t val f : T.t -> unit end
7+
The parameter cannot be eliminated in the result type.
8+
Please bind the argument to a module identifier.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
(* TEST
2+
subdirectories = "lib1 lib2";
3+
readonly_files = "lib1_client.ml lib2_client.ml";
4+
compile_only = "true";
5+
setup-ocamlopt.byte-build-env;
6+
7+
(* Set up the Lib modules that the client modules depend on *)
8+
all_modules = "lib1/lib.ml";
9+
ocamlopt.byte;
10+
all_modules = "lib2/lib.ml";
11+
ocamlopt.byte;
12+
13+
(* Compile Lib1_client against Lib1 *)
14+
flags = "-I lib1";
15+
all_modules = "lib1_client.ml";
16+
ocamlopt.byte;
17+
18+
(* Compile Lib2_client against Lib2 *)
19+
flags = "-I lib2";
20+
all_modules = "lib2_client.ml";
21+
ocamlopt_byte_exit_status = "2";
22+
ocamlopt.byte;
23+
check-ocamlopt.byte-output;
24+
*)
25+
26+
(* This test is a regression test. The bug was in the last step: the compiler crashed
27+
with an exception and backtrace instead of printing a useful error message. The
28+
issue was that the compiler was erroneously running in a mode where its error reporting
29+
is allowed to load cmi files from disk. This mode is undesirable because it means
30+
that the compiler can encounter new exceptions (e.g. that the new cmi file it loads
31+
is not consistent with other cmi files) while doing error reporting for the old
32+
exception.
33+
*)

typing/printtyp.ml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,20 @@ let wrap_printing_env ~error env f =
843843
if error then Env.without_cmis (wrap_printing_env env) f
844844
else wrap_printing_env env f
845845

846+
let wrap_printing_env_error env f =
847+
let wrap (loc : _ Location.loc) =
848+
{ loc with txt =
849+
(fun fmt -> Env.without_cmis (fun () -> loc.txt fmt) ())
850+
(* CR nroberts: See https://github.com/ocaml-flambda/flambda-backend/pull/2529
851+
for an explanation of why this has drifted from upstream. *)
852+
}
853+
in
854+
let err : Location.error = wrap_printing_env ~error:true env f in
855+
{ Location.kind = err.kind;
856+
main = wrap err.main;
857+
sub = List.map wrap err.sub;
858+
}
859+
846860
let rec lid_of_path = function
847861
Path.Pident id ->
848862
Longident.Lident (Ident.name id)

typing/printtyp.mli

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,14 @@ val wrap_printing_env: error:bool -> Env.t -> (unit -> 'a) -> 'a
5050
(* This affects all the printing functions below *)
5151
(* Also, if [~error:true], then disable the loading of cmis *)
5252

53+
(** [wrap_printing_env_error env f] ensures that all printing functions in a
54+
[Location.error] report are evaluated within the [wrap_printing_env
55+
~error:true env] context. (The original call to [f] is also evaluated
56+
within that context.)
57+
*)
58+
val wrap_printing_env_error :
59+
Env.t -> (unit -> Location.error) -> Location.error
60+
5361
module Naming_context: sig
5462
val enable: bool -> unit
5563
(** When contextual names are enabled, the mapping between identifiers

typing/typecore.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10255,7 +10255,7 @@ let report_error ~loc env = function
1025510255
automatically if ommitted. It cannot be passed with '?'.@]" label
1025610256
1025710257
let report_error ~loc env err =
10258-
Printtyp.wrap_printing_env ~error:true env
10258+
Printtyp.wrap_printing_env_error env
1025910259
(fun () -> report_error ~loc env err)
1026010260
1026110261
let () =

typing/typemod.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3869,7 +3869,7 @@ let report_error ~loc _env = function
38693869
"Cannot compile an implementation with -as-parameter."
38703870

38713871
let report_error env ~loc err =
3872-
Printtyp.wrap_printing_env ~error:true env
3872+
Printtyp.wrap_printing_env_error env
38733873
(fun () -> report_error env ~loc err)
38743874

38753875
let () =

0 commit comments

Comments
 (0)