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

Weird behaviour for pack mlis, breaking on 4.04 #128

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AltGr
Copy link
Member

@AltGr AltGr commented Nov 28, 2016

I had a weird error while building dose3 on 4.04, and tracked it down
to this. Disabling the hack seemed to work for me, but it must have
been here for a reason ?

I was getting:

Warning 58: no cmx file was found in path for module Common, and its interface was not compiled with -opaque
+ touch algo/algo.mli  ; if  ocamlfind ocamlopt -pack -I algo algo/defaultgraphs.cmx algo/diagnostic.cmx algo/depsolver_int.cmx algo/depsolver.cmx algo/flatten.cmx algo/statistics.cmx algo/dominators.cmx algo/strongdeps.cmx algo/strongconflicts_int.cmx algo/strongconflicts.cmx -o algo/algo.cmx  ; then  rm -f algo/algo.mli  ; else  rm -f algo/algo.mli  ; exit 1; fi
File "algo/algo.cmx", line 1:
Error: The implementation (obtained by packing)
       does not match the interface algo/algo.mli:
       ...
       In module Defaultgraphs.SyntacticDependencyGraph.G:
       Type declarations do not match:
          type t =
             Graph.Imperative.Digraph.ConcreteLabeled(Defaultgraphs.SyntacticDependencyGraph.PkgV)(Defaultgraphs.SyntacticDependencyGraph.PkgE).t
       is not included in
          type t = Graph.Imperative.Digraph.ConcreteLabeled(PkgV)(PkgE).t
Command exited with code 1.

Not creating a temporary, empty .mli worked better...

edit: forgot to mention that there was no mli for Defaultgraphs, which might be of importance

Don't know what it's for but it breaks dose3 compilation on 4.04
@gasche
Copy link
Member

gasche commented Dec 26, 2016

@AltGr, I'm looking at this again and this is very strange. Here is the command-line produced by legacy ocamlbuild (before your patch) (reindented for clarity):

touch algo/algo.mli 
if  ocamlfind ocamlopt -pack -I algo algo/defaultgraphs.cmx algo/diagnostic.cmx algo/depsolver_int.cmx algo/depsolver.cmx algo/flatten.cmx algo/statistics.cmx algo/dominators.cmx algo/strongdeps.cmx algo/strongconflicts_int.cmx algo/strongconflicts.cmx -o algo/algo.cmx
then
   rm -f algo/algo.mli
else
   rm -f algo/algo.mli
   exit 1
fi

After your patch, it only does the central command:

ocamlfind ocamlopt -pack -I algo algo/defaultgraphs.cmx algo/diagnostic.cmx algo/depsolver_int.cmx algo/depsolver.cmx algo/flatten.cmx algo/statistics.cmx algo/dominators.cmx algo/strongdeps.cmx algo/strongconflicts_int.cmx algo/strongconflicts.cmx -o algo/algo.cmx

The question is then: why would the presence or not of a .mli file (not a .cmi file, a .mli file) influence the behavior of the pack build? The answer is that, indeed, the code in the type-checker that computes the signature of a packed module (to produce the corresponding .cmi) does depend on whether a .mli file is present or not: https://github.com/ocaml/ocaml/blob/10e5659/typing/typemod.ml#L1709.

(Note: this .mli file is not used to compile the signature of the packed module -- otherwise the resulting packed module would have an empty interface, meaning that its submodules are private and cannot be accessed. It would break compilation. The empty .mli files is never compiled, so it never produces an empty .cmi file.)

I don't understand yet the details of why the workaround used to work before 4.04 and why it was thought helpful.

@gasche
Copy link
Member

gasche commented Dec 26, 2016

So the idea is as follows.

When foo.mli does not exist, producing either foo.cmo or foo.cmx will produce foo.cmi at the same time. To avoid concurrency races on foo.cmi when building both foo.cmo and foo.cmx, ocamlbuild serializes this process by making the .cmx rule depend on the .cmi -- which forces doing a bytecode build first that produces the cmi.

In term of packed modules, that means that native pack .cmx is always built in presence of a .cmi file that was produced by builting the same pack with the bytecode compiler. When the .cmx pack command runs, there is thus a .cmi file (and we consider the case where there is no corresponding .mli file in the project source).

The type-checker takes a different path depending on whether a .mli file is present or not. When it is present, its signature is read and reused: the .cmx is produced with the type information read from the .cmi coming from the bytecode compilation. When no .mli file is present, the signature is re-computed from the packed module signatures.

Creating an empty .mli file has the effect of always reusing the .cmi signature of the bytecode compilation. In fact, the code path is slightly more complex: a signature is first inferred from the packed modules, and checked against the .cmi signature; the check produces a coercion which is probably the "no change required" (in the general case two signatures may be compatible but moving from one to another may require reordering or dropping stuff), but may be a more complex signature that is equivalent to the identity. In the code path without a .mli, the signature inferred from the modules is reused without any check (so the "no change required" coercion is used).

I suspect that the difference in treatment comes from this.

@gasche
Copy link
Member

gasche commented Dec 26, 2016

@AltGr I cannot reproduce the issue with dose3 5.0.1. Which version should I use to be able to reproduce the issue?

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

Successfully merging this pull request may close these issues.

2 participants