Menhir: Incorrect module dependency with type inference

[bbc2]

My parser is in a foo library, wrapped as Foo by Dune. With a particular grammar, Menhir prefixes a reference to my Ast module with Foo.. This is invalid because the parser itself is in the foo library .

Here is a minimal example:

.
├── ast.ml
├── dune
├── dune-project
└── parser.mly

parser.mly:

%token Return
%token Eof

%start <Ast.Block.t> block

%%

let block :=
  | ~ = retstat; Eof; <Some>

let retstat :=
  | Return; {[]}

%%

ast.ml:

module Retstat = struct
  type t = int list
  [@@deriving eq,ord,show]
end

module Block = struct
  type t = Retstat.t option
  [@@deriving eq,ord,show]
end

dune:

(menhir
 (modules parser))

(library
 (name foo))

dune-project:

(lang dune 1.10)
(using fmt 1.1 (enabled_for dune))
(using menhir 2.0)

This results in the following error:

> dune build @check
Module Parser in directory _build/default depends on Foo.
This doesn't make sense to me.

Foo is the main module of the library and is the only module exposed 
outside of the library. Consequently, it should be the one depending 
on all the other modules in the library.

Dune uses --infer-write-query and --infer-read-reply on Menhir to infer the types from the grammar. As expected, it also uses ocamlc.opt -short-paths -i ... to infer the types on the generated mock. However, the -short-paths option, added in #1743 to fix #1504, doesn't seem to be enough to avoid the unwanted reference to the enclosing Foo module.

I've derived a small example from the mock that Menhir generated:

let (xv_retstat, xv_block) =
  let _f (retstat : 'tv_retstat) = (Some retstat : Ast.Block.t) in
  (raise Not_found : 'tv_retstat * Ast.Block.t)

This results in:

> ocamlc.opt -I .foo.objs/byte -open Foo -short-paths -i -impl parser__mock.ml.mock
val xv_retstat : Foo.Ast.Retstat.t
val xv_block : Ast.Block.t

Issue originally submitted at https://gitlab.inria.fr/fpottier/menhir/issues/26.

[ghost]

Yeah this is a known issue. I just had a look at the original issue. @fpottier, the -open Foo is actually necessary: it contains the aliases of the form: module X = Foo__X. If we removed it, then users would have to write Foo__X instead of just X. Thish is something we definitely don't want given that these names are an implementation detail until OCaml get proper namespaces.

The only workaround I know of is to add a dummy Foo module at the beginning of the parser:

module Foo = struct end

to prevent OCaml from generating references to Foo.X.

[grayswandyr]

Hi @diml unfortunately it doesn't seem to be enough (?). In my case (about which I had already filed an issue), doing this yields the following warning if I update Parser.mly and Smv_trace_parser.mly in src/ with module Libelectrod = struct end:

$ dune build @install 
File "src/Parser__mock.ml.pp.mock", line 1:
Warning 63: The printed interface differs from the inferred interface.
The inferred interface contained items which could not be printed
properly due to name collisions between identifiers.
File "src/Parser.mly", line 18, characters 0-31:
  Definition of module Libelectrod/1
File "_none_", line 1:
  Definition of module Libelectrod/2
Beware that this warning is purely informational and will not catch
all instances of erroneous printed interface.

Any idea?

[ghost]

Does the build succeed? If yes, you can safely disable this warning.

[grayswandyr]

Yes thanks.

[ghost]

BTW, we had a chat about this problem with @fpottier during ICFP this year. Unfortunately, there is no simple solution to this problem. The root issue here is that we are doing something weird in Dune with these __ modules and we do this because we don't really have much choice. Hopefully, in the future we can clean this up with better language support. However, this will take time.

So for now the best to do is to be patient and accept that things don't work as perfectly as they could.

[grayswandyr]

No worries. Using dune is such an improvement that I can endure a couple of tweaks, as long as I know the corresponding warnings can be safely ignored. What's more, thanks to your suggestion, I was able to remove those lines preventing Menhir to rely on type inference in the dune file, so that's cool.
Thanks for the good work!

[rgrinberg]

As explained, this is a limitation of menhir and not dune.