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

Wrong module type signature grabbed when using packed libraries #6433

Closed
vicuna opened this issue May 19, 2014 · 6 comments
Closed

Wrong module type signature grabbed when using packed libraries #6433

vicuna opened this issue May 19, 2014 · 6 comments

Comments

@vicuna
Copy link

vicuna commented May 19, 2014

Original bug ID: 6433
Reporter: @mmottl
Assigned to: @garrigue
Status: acknowledged (set by @garrigue on 2014-05-20T02:55:29Z)
Resolution: open
Priority: normal
Severity: feature
Version: 4.01.0
Target version: undecided
Category: typing
Duplicate of: #6305
Monitored by: @gasche @hcarty @Chris00 @mmottl

Bug description

I'm not sure to which extent this qualifies as a bug, but it's at least annoying behavior:

Lets assume you want to pack several modules into a library module "Foo", e.g. module "Utils":

ocamlopt -c -for-pack Foo utils.ml

Some functionality in the packed modules may only be used internally so you may want to hide it by defining an interface file "foo.mli", which would be compiled as usual:

ocamlopt -c foo.mli

Finally, we pack and create the library "Foo":

ocamlopt -pack utils.cmx -o foo.cmx
ocamlopt -a foo.cmx -o foo.cmxa

Here is the problem: lets assume the developer wants to export everything in module "Utils". Then a seemingly elegant way to do this would be to put the following in the above-mentioned file "foo.mli":

module Utils : module type of Utils

But this has unintended consequences. If a user of library "Foo" now also defines a module "Utils" in their project, the compiler may complain that there are "inconsistent assumptions" about module "Utils".

The apparent reason is that "module type of Utils" in "foo.mli" is now being resolved using the "Utils" interface in the end user's project rather than the one in the library. This is a little strange, because if the end user doesn't define a "Utils" module, then the compiler will correctly refer to the "Utils" interface in the packed library - as intended.

I would say there is no scenario where it would make sense to interpret "module type of" in the above way. It's clear during packing what the referred to signature is.

A workaround for the library developer would be to just copy the whole signature of module "Utils" verbatim into "foo.mli", but this is obviously a cumbersome solution.

Could the semantics of name resolution in this scenario please be changed so that "module type of" refers to the correct context? Thanks!

@vicuna
Copy link
Author

vicuna commented May 20, 2014

Comment author: @garrigue

While I agree that this behavior is problematic, omitting the reference to Utils has some consequences.
Let's see what is happening: when compiling foo.mli, we open utils.cmi to read its signature.
As a result, utils.cmi's digest gets included in foo.cmi, hence the conflict.
The problem is that, when we compile foo.mli, we have no idea that the goal is to make
a packed module. If we're not building a packed module, the final program may contain
Utils too, and it would be strange to allow that the signature for Utils in Foo is not the real
signature of Utils.
Note also that this is only part of the problem: assume that Utils refers to Aux, which is
also part of Foo. You would expect the references to switch to Foo.Aux, but you have no way
to do that at the mli level.

However I don't see any other good solution. So the trivial one may be the right one:
when typechecking "module type of Utils", and Utils is a persistent module (i.e. a cmi),
register the dependencies of Utils but not utils.cmi itself. This makes sense, because
"module type of" does not strengthen the module type it produces, meaning that
this type is not really the type of Utils, but just something that is textually identical.
While the absence of coherence check might be surprising at times, I believe this is sound.

@vicuna
Copy link
Author

vicuna commented May 20, 2014

Comment author: @mmottl

It seems convincing that there wouldn't be any soundness issue here, since "manual" textual substitution also solves the problem. The generated "foo.cmi" already contains all required signature information so if I understand this correctly, it should really just be a matter of not adding the utils.cmi dependency when "module type of Utils" is being used. Of course, other uses of "Utils" in the signature that do not refer to the exported "Utils"-module would still introduce such dependencies.

As you said, in other (i.e. non-packing) contexts the coherence check might still be expected even with "module type of". Maybe this could be solved by a compiler flag that indicates that compiling "foo.mli" will produce the signature of a packed module. It would be trivial for the developer to add this to the build process, but I'm not sure the extra complexity is justified.

@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@garrigue
Copy link
Contributor

As discussed, the fix is to have some special handling for module type of A when A is a persistent module, so that not dependency on a.cmi is added.
While this should work, this also means that we rely explicitly on the current behaviour of module type of, which doesn't strengthen the module type (otherwise, there would be references to the original A in the module type).
If there is an agreement on this solution, I'm ready to implement it.

@mmottl
Copy link
Contributor

mmottl commented May 13, 2020

I believe it should be fine to make this incremental improvement. I faintly remember there was some discussion of changing the semantics of module type of. Is this still relevant? It's always possible to strengthen a module signature using the module type of struct include ... end trick anyway.

@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

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

No branches or pull requests

3 participants