search

ocaml / ocaml

The core OCaml system: compilers, runtime system, base libraries
https://ocaml.org
Other
5.19k stars 1.06k forks source link

Some type error printing erroneously loads cmis #13098

Closed ncik-roberts closed 1 month ago

ncik-roberts commented 1 month ago

Printing a module type error can load cmis. This is a bug for incremental builds.

It's possible to write a test that observes the bad behavior, as loading cmis can cause a further exception to be raised if the cmi is not consistent with other cmis. Such an error can present the user with an uninformative backtrace instead of a useful error message.

See the test case added in ncik-roberts/ocaml at commit 4bd59d6a8d583b05be74e02ca1f7d848cf589bed for an example reproduction. In essence, the reproduction turns the erroneous cmi-loading into a compiler crash by making the type error involve a module named Lib1_client__X, where lib1_client.cmi is a cmi file inconsistent witth the ongoing compilation. (Lib1_client__X is meant to confuse the error printer's heuristic to treat Foo__Bar as Foo.Bar when these paths are aliases.)

Here's a snippet of the garbled error message, to help people searching the issue tracker:

Error: This functor has type "Fatal error: exception Persistent_env.Error(_)
Raised at Persistent_env.error in file "typing/persistent_env.ml", line 32, characters 16-33
Called from Persistent_env.acknowledge_pers_struct in file "typing/persistent_env.ml", lines 198-200, characters 2-4
Called from Persistent_env.find_pers_struct in file "typing/persistent_env.ml", line 233, characters 17-64
Called from Persistent_env.find in file "typing/persistent_env.ml", line 272, characters 6-55
...

You may be surprised about this, because Typemod.report_error appears to suppress loading of cmis by wrapping the error reporting code with Printtyp.wrap_printing_env ~error:true: https://github.com/ocaml/ocaml/blob/5.1.1/typing/typemod.ml#L3418

However, the problem is that the wrapped call to report_error doesn't do the printing itself — it constructs a value which contains several Format.formatter -> unit functions. Thus, Printtyp.wrap_printing_env ~error:true just wraps the creation of this value, not also the contained Format.formatter -> unit functions which are later run in a different context.

The error printing in typecore.ml has the same code structure as typemod.ml, so it may have the same issue. (I couldn't easily find a reproduction though.)

Octachron commented 1 month ago

The printing type errors involves even more mutable states that needs to be handled painstakingly, so it is quite possible that this path has been hardened against this error.