Add missing interfaces

[shindere]

Studying the build system shows that the list of warnings used to compile
the different components of the compiler is not always the same.

That made me curious about which warnings we really need an which we don't.

Thus, the first commit in this pull request, which is not intended to
be merged, reveals all the warnings.

On this basis, I started to try to eliminate the warning about missing
interfaces. It may look useless to fix warnings, but the idea is that
if a state is reached where a warning does not appear any longer,
then we can re-enable it and that warning can become useful again
to test the quality of code that will be added after it has been enabled.

So, the core of this pull request, which does intent to be merged
once completed and cleaned up, consists in adding .mli files.

At this stage, only 3 occurrences of warning 70 remain, in the
native backend, for the modules arch, CSE and branch_relaxation_intf. The
last commit is a failed attempt
to disable the warning for branch_relaxation_intf.

Since this attempt does not work, would it be okay to create
an interface, although it will be redundant with the implementation?

Another approach would be to leave this module as is, since
the future rules used to build the compiler will have room for
per-module flags, so it will be possible to pass compiler-specific
options to the compiler. But that would still be an irregularity.

Regarding arch and CSE, I started to look at the different interfaces
and, although there seem to be some differences, it may still be
possible to come up with a common interface. If that's okay I would be
happy to investigate in this direction.

Ultimately, this PR could conclude with a commit re-enabling warning
70, and then other warnings could be fixed and re-enabled in the same way.

Of course the goal here is not to fix all warnings. Warning 4, for
instance, does nhot look like a warning one may want to fix in a
systematic way.

[xavierleroy]

OK, I'll bite:

What's wrong with .ml files without .mli interfaces?

I know some people don't like that and that's why we have an optional warning for this, but in the context of the core OCaml system I don't see much harm.

[shindere]

Xavier Leroy (2022/06/01 10:59 -0700):

OK, I'll bite: What's wrong with .ml files without .mli interfaces? I know some people don't like that and that's why we have an optional warning for this, but in the context of the core OCaml system I don't see much harm.

There is perhaps not much harm not to have them, but I think there is much to be gain in having them. It's an invitation to write specifications and to have a clear distinction between what's private and what's public. To me, a codebase with MLI files is just more easy to get into for newcomers. And it's good that our code gives good examples, in my opinion. There is also that, in my opinion, it's nice to have some consistency. The idea here would be to at least revert the default. For now by default we don't complain about missing interfaces and perhaps in some cases it would have really helped to have one. Take the case of `typing/cmt2annot`. We would swich to a model where the default owuld be to have interfaces (and complaints if ou don't) and the possibility to override that default but with having to be explicit about that.

[bluddy]

I do wish there was some other way to indicate private and public elements of a module. OCaml has a schizophrenic approach here. Haskell and Rust require signatures on their functions, mostly because of their inability to infer sufficiently. OCaml insists on only allowing types that can be fully inferred by default so you don't have to annotate anything, and yet 'good practices' requires writing out the full types, in a separate file, no less (requiring synchronization). Maybe I wouldn't mind it so much if I could just mark a function with [@@export] and let the compiler update the signature in-place in the mli for me. But then again, if I could do that, why not just use the annotation to specify the interface without an mli file?

[xavierleroy]

It's an invitation to write specifications and to have a clear distinction between what's private and what's public.

I agree 100% for library modules or application modules that have a well-defined interface (pun intended!) with the rest of the application. But, to take an example, ocamldoc doesn't fit this pattern and will probably be replaced by something else before anyone writes any documentation on its modules. Just adding a .mli containing the output of ocamlc -i doesn't help much by itself.

[shindere]

Xavier Leroy (2022/06/02 10:44 -0700):

I agree 100% for library modules or application modules that have a well-defined interface (pun intended!) with the rest of the application. But, to take an example, ocamldoc doesn't fit this pattern and will probably be replaced by something else before anyone writes any documentation on its modules. Just adding a .mli containing the output of ocamlc -i doesn't help much by itself.

Perhaps not "in itself", indeed, although even for that I am not sure. To me, the fact that the interface is there means it can be refined, the step to do so will be easier. The same holds for main modules. One may wonder what it brings to provide an interface to them. In short term, apart from making things more regular, not much, perhaps. But if one imagines that one day these tools can be used as libraries, then, certainly, it will become handy to have aninterface file for their entry points, too.

[shindere]

The code has been rebased and updated and the story has been made
complete and (hopefully) coherent, so this PR is now ready to be
reviewed.

It consists of 40 commits. The first commit enables warning 70 about
missing interfaces so that all warnings of this kind can be seen. The 38
next commits introduce
the missing interfaces. Finally, the 40th commit turns warning 70
into an error.

Regarding ocamldoc, the introduced interface files have been minimized.
This allowed to detect a few unused values, types, constructors and
exceptions that have also been removed (many thanks to @Octachron for
having pushed me to go beyond the automatically generated interfaces,
the result feels much more satisfactory that way).

Regarding the motivation of this PR, interface files become
useful in the context of out-of-source-tree builds and also make
it possible to parallelize builds evenmore.

Regarding out-of-source-tree builds, is a module has no interface file,
then compiling its implementation will produce a .cmo file
which will be added to the build tree but, if no care is taken, the .cmi
file produced as a side-effect wiill go to the source tree.

It would certainly be possible to extend the compiler's build system
to take special care and put these .cmi files generated while compiling
.ml files at the right place, but this would make our already-complicated
build system even more complex, for no good, it seems.

About parallel builds: one may want to build a bytecode and a native
version of a program simultaneously. If an implementaiton-only module
is simultaneously compiled by ocamlc and ocamlopt, then the two compilers
will compeet to produce a .cmi file, which does not seem desirable.

Finally, this PR revealed that we still build the cmt2annot tool
which we do not install any longer. How about getting rid of it completely?

[damiendoligez]

What's wrong with .ml files without .mli interfaces?

See #10364 (comment) for an example of annoyance caused by a missing .mli.

[shindere]

This is currently going through precheck, see build #714.

[shindere]

Rebased on latest trunk

[damiendoligez]

I see you've renamed branch_relaxation_intf.ml to branch_relaxation_intf.mli, thus getting an .mli without a corresponding .ml. Are you sure this will not cause problems down the line? Note that the dune file mentions branch_relaxation_intf as a module name.

Otherwise this looks good modulo a few typos.

[damiendoligez]

Do you need to export main? What is the difference with driver/main.mli and driver/optmain.mli?

[damiendoligez]

Suggested change

	This is because other modules yse the let module construct ot access it
	This is because other modules use the let module construct to access it

[damiendoligez]

Suggested change

	code during initialisation and does not export any funciton *)
	code during initialisation and does not export any function *)

[damiendoligez]

Suggested change

	(* Interface to expunge's main entry poit *)
	(* Interface to expunge's main entry point *)

[damiendoligez]

Again, I don't understand why you need to export this.

[shindere]

Many thanks for your so careful review Damien, as usual. Damien Doligez (2022/06/27 10:13 -0700):

@damiendoligez approved this pull request.

Thanks. I will wiat for @Octachron's blessing, too.

I see you've renamed branch_relaxation_intf.ml to branch_relaxation_intf.mli, thus getting an .mli without a corresponding .ml. Are you sure this will not cause problems down the line?

Yes.

Note that the dune file mentions branch_relaxation_intf as a module name.

I don't know how to update that dune file. Also, I think we agreed that when updating the compiler's build system the dune files do not need to be updated, that they are taken care of by those persons actually using them, so I didn't even try to update them.

Otherwise this looks good modulo a few typos.

Which I fixed, many thanks for that.

In [1]lex/main.mli: +val main : unit -> unit +(** The entry point for ocamllex *) Do you need to export main?

My policy was that, if a `.ml` file provides a main function, then I export it. My motivation for doing that was being future-proof, kind of. For instance if one day one wishes to call the entry point from a library, to make this use possible.

What is the difference with driver/main.mli and driver/optmain.mli?

If you look at the corresponding .ml files, you will see they don't have any main function and just execute code during their initialisation, so there simply is nothing to export, at the moment. The same goes for `expunge` about which you asked, too. If we one day wnat to be able to use all the tools as libraries, I think it may be nice to make sure their main entry point is exported and this entry point does not exit. At the moment I think this does not make a big difference because we have states that make it impossible to run an entry point more than once in a process but hopefuly that will change. Remains the question of what to do with `cmt2annot`.

[shindere]

Florian Angeletti (2022/06/28 05:36 -0700):

In [1]ocamldoc/odoc_test.mli: This file is only used by the testsuite. Ultimately it should be moved to testsuite/tests/tool-ocamldoc. For this PR, it is probably fine to have an empty mli file.

No problem. I mentionned in the MLI file that it should be moved to the testsuite, per your suggestion.

[shindere]

Florian Angeletti (2022/06/28 05:56 -0700):

@Octachron commented on this pull request. ══════════════════════════════════════════════════════════════════════════ In [1]ocamldoc/generators/odoc_literate.mli: Since the generator is inheriting the class Odoc_html.Generator.html without adding any public method, you can simplify that class type to: class html : Odoc_html.Generator.html

That's very nice simplification and clarification of the code. Many thanks for the suggestion, which has just been implemented.

[shindere]

Following an off-line suggestion by @dra27 (thanks!), I checked that all the
added interfaces get installed correctly by make install when they should.

For those interfaces outside of the ocamldoc directory things were
already working, but it was not the case for the ocamldoc interfaces.

The commits have now been installed to install most of them.

Those which are not installed are odoc.mli which is empty, likewise
for odoc_test.ml and those of the two generators, odoc_todo.mli
and odoc_literate.mli.

[Octachron]

Overall, I rather like to have interface file available in a fully uniform way.

The most thorny changes are the new interface files for ocamldoc's generators which are half-public interfaces which were never written explicitly. Consequently, preserving backward-compatibility for ocamldoc generators make those interface files quite messy. However, the current state of this PR does seem like a seem sensible compromise to avoid pouring too much effort and time on ocamldoc.
I am thus also approving this PR.

[shindere]

Thanks for the blessing, the positive feedbakc and all the time you have invested in helping me to improve this work. In my opinon, your suggestions really made a big difference between the initial state of the PR and what has finally just been merged. So, many thanks again.