Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.mli only modules #9

Closed
rgrinberg opened this issue Feb 28, 2017 · 24 comments
Closed

.mli only modules #9

rgrinberg opened this issue Feb 28, 2017 · 24 comments

Comments

@rgrinberg
Copy link
Member

When porting cohttp to jbuilder, I discovered that .mli only modules don't seem to work. Since ocamlbuild supports them, it would be great if jbuilder would as well.

@ghost ghost closed this as completed in 9511320 Feb 28, 2017
@ghost
Copy link

ghost commented Feb 28, 2017

Properly supporting .mli could only work for unwrapped libraries since you can't alias a module without implementation, so I'm not sure it's worth doing. Moreover I heard an argument in the past that .mli only modules are not a good idea (/cc @sliquister).

However, to make porting easier, I added a hack to automatically copy the .mli to the .ml in such cases, with a warning.

@v-gb
Copy link

v-gb commented Feb 28, 2017

The argument against .mli only file is simple:

  • they are useless. Anything you can do with the .mli, you can do instead with .ml files with no downside that I know of
  • supporting them makes other things error prone. If you have a .ml and a .mli, and rename only one of them, now you have a module that exposes its internals which is not an error, and a .mli without .ml which is not an error either
  • oftentimes, you're better off using a .ml where you can use [@@deriving sexp_of, compare] on types defined in there

I'll also point out copying the .ml to a .mli doesn't work for the case where the .mli contains:

module F(X : ..) : sig module type S = .. end

and some other file uses F(X).S or F(X).t (which doesn't create a dependency on the implementation), which I have seen people use.

@ghost
Copy link

ghost commented Feb 28, 2017

Thanks for the details. I've updated the warning message to point to this discussion

@Drup
Copy link

Drup commented Apr 9, 2017

For what it's worth, I disagree strongly, and I find .mli alone very useful. It's a feature of the OCaml compiler, and I don't see why an OCaml build system would not handle that.

they are useless. Anything you can do with the .mli, you can do instead with .ml files with no downside that I know of

Not for documentation purposes. Documentations is better hosted in an .mli (tools handle it better, users are used to it, ...).

supporting them makes other things error prone.

.mli alone only works if it only contains types. If that is the case, the .ml either only contain types (and was thus useless) or contained implementation not-exported, which are then useless. In the worse case, you will have two modules with the same name, and then everything is going to explode either way. That situation is never a real problem.

oftentimes, you're better off using a .ml where you can use [@@deriving sexp_of, compare] on types defined in there

That's my choice, isn't it? :)

@Drup
Copy link

Drup commented Apr 9, 2017

(Note that I agree with the problem of aliasing modules with no implementation ... but I still consider that a bug in the compiler ....)

@rgrinberg
Copy link
Member Author

Drup raises a good point here about installation. If we're unable to have .mli modules, is there a way to mark an analogous .ml file to be installed?

@ghost
Copy link

ghost commented Apr 10, 2017

@rgrinberg I'm pretty sure it's what jbuilder does already

@ghost
Copy link

ghost commented May 4, 2017

This seems to come up often, so I think something could be done about it.

Currently .mli only modules work by chance. The facts that you cannot alias them and that if you write exception E you'll get a cryptic error much later only when you link an executable are major issues.

It'd be different if they where properly supported by the compiler. I think we need two things:

  1. a flag to tell the compiler that a .mli file has no corresponding implementation, so that it can check that there are no values declared in it and properly fail otherwise
  2. the ability to alias such modules without creating a dependency on their implementation or a way to produce an empty implementation

If someone is willing to work on this, I'm happy to add proper support for them in jbuilder

@dra27
Copy link
Member

dra27 commented May 26, 2017

Just for info, porting opam itself to use jbuilder, I hit an instance of this little heuristic not working in src/format/opamTypes.mli which includes the line include module type of struct include OpamParserTypes end which is not valid in an .ml file.

@ghost
Copy link

ghost commented May 26, 2017

Maybe a better heuristic would be to generate:

module rec Foo : sig
  <contents of the mli>
end = Foo
include FOo

@dra27
Copy link
Member

dra27 commented May 26, 2017

Who'd have thought recursive modules could get into something so superficially simple! It'd certainly be handy for it to work automagically

I'm not sure where I stand on not being allowed to use (or strongly discouraged from using) .mli files solely for collecting types together (it's done extensively in opam, but in many ways I find it quite irritating) - it'll certainly be easier to adopt in this instance if jbuilder can work without needing to adapt the .mli file from day 1.

@ghost
Copy link

ghost commented May 26, 2017

The problem with this solution is that if you write exception E by mistake in your mli then you get a error message that is impossible to understand... Though I'm happy to add it if we have a compiler flag to check that a .mli doesn't need an implementation

@dra27
Copy link
Member

dra27 commented May 26, 2017

Might it be possible (if dirty) to make the shim only available in the wrapped false case - i.e. for legacy libraries only?

@dra27
Copy link
Member

dra27 commented May 27, 2017

I haven't double-checked, but I think I hit a related problem with this in #104 - if you're using an external library where there's a .cmi file only, this creates a dependency on a non-existent module which prevents using -linkall (which is a problem if compiling toploops).

@ghost
Copy link

ghost commented May 29, 2017

This is weird, what do you mean by creating a dependency? jbuilder will only pass .cmxa files when linking, so I can't see how this could happen. Except of course if in the external library the .cmi only module wasn't really a pure interface, which enforces the idea that mli only modules are a bad idea :p

Anyway, I added a stanza (mli_of_ml (foo bar ...)) which produces rules to generate foo.ml, bar.ml... using the recursive module trick. This should make it easy to convert project using mli only modules and will make it explicit to the user what's happening.

@dra27
Copy link
Member

dra27 commented May 29, 2017

@diml - here I think is a minimal and correct example. So I have a third-party library consisting of two files LibTypes.mli:

type t = A

and LibMod.ml:

let foo = LibTypes.A

and that's been compiled using ocamlc -a -o lib.cma LibTypes.mli LibMod.ml

Now I have a library that's part of my project which also includes two files ProgTypes.mli:

type u = B

include module type of struct include LibTypes end

and ProgMod.ml:

let foo = LibTypes.A
let bar = ProgTypes.B

Now, without jbuilder that's compiled using ocamlc -a -o proglib.cma ProgTypes.mli ProgMod.ml.

I can now make a toplevel with these: ocamlmktop -o mytop.exe lib.cma proglib.cma

So far, so good. Now, let's convert proglib.cma to be built using jbuider. ProgTypes.mli doesn't translate, and so I make ProgTypes.ml to be:

type u = B

include LibTypes

and jbuilder now includes module ProgTypes in proglib.cma. Now I build my toplevel:

[opam:default] C:\DRA\test\jbuilder>ocamlmktop -o mytop.exe lib.cma proglib.cma
File "_none_", line 1:
Error: Error while linking proglib.cma(ProgTypes):
Reference to undefined global `LibTypes'

at this point, in order to build this with jbuilder, I'm forced to vendor the third-party library too. That's not fixed using the recursive module trick either.

@ghost
Copy link

ghost commented May 29, 2017

I see, can you try the (ml_of_mli (ProgTypes)) with the development version of jbuilder?

@dra27
Copy link
Member

dra27 commented May 30, 2017

That seems to be working - I think I must have set something up incorrectly in my previous test with the recursive module trick. Thanks!

@kit-ty-kate
Copy link
Member

Does the ml_of_mli function still exist ? It doesn't seem to work out of the box.

@dra27
Copy link
Member

dra27 commented Jul 31, 2017

No - see fd76d7b.

@dra27
Copy link
Member

dra27 commented Jul 31, 2017

See, for example, here for the current workaround

@kit-ty-kate
Copy link
Member

Thanks. I saw the workaround but I was wondering why it wasn't the default behaviour ? (without a warning I mean)
It's quite annoying if there are several modules in this case.

@dra27
Copy link
Member

dra27 commented Jul 31, 2017

Jbuilder is opinionated 😄 With jbuilder automatically handling link commands (i.e. never having to having to specify the module name for a types-only module in a Makefile list), this doesn't bother me for new projects. I agree for legacy libraries it's a nuisance, but I also agree it's better to wait until there's proper compiler support for .mli-only modules

@Drup
Copy link

Drup commented Aug 2, 2017

The recursive module hack doesn't work in general. With tyxml's xml_sigs, the compiler gives you:

Error: Cannot safely evaluate the definition
       of the recursively-defined module HACK

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants