%import interface types with deriving

[Leandros]

If I try to derive eq from imported types, I get the following error message:

Error: eq cannot be derived for fully abstract types

The setup is as follows:

mod.mli

type foo = Bar of int
[@@deriving eq,show]

mod.ml

type foo = [%import: Mod.foo]
[@@deriving eq,show]

dune

; ...
  (preprocess
    (staged_pps
      ppx_import
      ppx_deriving.std
; ...

Works fine if I import types from other interfaces/modules.
Any ideas?

[gasche]

I cannot reproduce the issue with ocaml.4.07.1, ppx_deriving.4.2.1 and ppx_import.1.6. The reproduction command I use is

ocamlfind ocamlc -package ppx_import -package ppx_deriving.std -dsource -c mod.ml

[Leandros]

Here is a repro with dune.
repro.tar.gz

% dune build repro.a                                                                                                                                                                                                             INSERT ✗ 127
    ocamldep .repro.objs/foo.ml.d (exit 2)
(cd _build/default && /Users/leandros/.opam/4.07.1/bin/ocamldep.opt -modules -ppx '.ppx/c7f563a7dc0bec0315fe197895df9f4c/ppx.exe --as-ppx --cookie '\''library-name="repro"'\''' -impl foo.ml) > _build/default/.repro.objs/foo.ml.d
File "foo.ml", line 2, characters 0-46:
Error: eq cannot be derived for fully abstract types

[gasche]

Looking at _build/log, the problem arises when calling ocamldep to compute foo.ml.d; your usage pattern (depending on the same module being compiled) can only work if foo.cmi is already built when the implementation is preprocessed (otherwise ppx_import will not find the definition of Foo.t), this works correctly if you compile foo.mli before you try to process foo.ml, but dune first builds the dependencies of both files before compiling any of them, so this ordering does not work.

The take-away for me is that this particular usage pattern (using ppx_import to import the interface of a module from its own implementation) is not supported by dune (cc @diml), and I suspect most other build systems (it's natural to try to compile foo.ml.d before foo.cmi is available). I have not checked but I suppose it used to work with older versions of OCaml and ppx_import, which did not use the typing environment to resolve ppx_import queries (the change to the ocaml stdlib namespacing in 4.07 forced us to move to using the type-checker, which is conceptually the better approach).

[Leandros]

Hmm. Bummer. Thanks.

[ejgallego]

Yup, that's a pain; maybe you could reorganize code to put the necessary definitions on Mod_intf.ml ? and then import this module on both your files?

[ghost]

This has nothing to do with dune, it has to do with ppx_deriving_eq. When called from ocamldep, ppx_import replaces type t = [%import ...] by type t and [@@deriving eq] doesn't work on abstract types. cf: https://github.com/ocaml-ppx/ppx_import/blob/master/src/ppx_import.ml#L340

[gasche]

Thanks for noticing the ocamldep special case, I hadn't seen that indeed. (I linked the issue to the build system because manual testing doesn't call ocamldep at all, and works just fine.)

@diml, do you think that we should fix the issue by also propagating a special-casing of ocamldep in ppx_deriving itself? That seems ugly to me, and also the most obvious behavior (not generating any code for the annotations) may erase real dependencies (for example json derivers should incur a dependency on a Json module).

[ghost]

Well, i don't really see any other way. But if I'm honest, ppx rewriters simply shouldn't rely on the typer. Even though it is possible to do it, this is not a feature that was properly thought through, well tested, etc and there are a lot of corner cases that can't be solved in the existing system.

However, it would possible to design a ppx2 system with a clear scope that would interact properly with typing. Hint, hint, that seems like a good project for the OSF and it would definitely benefit the community :)

[gasche]

I agree the current state is not optimal; you were involved in the discussion to switch to the typer in ppx_import to avoid namespaces/module-aliases issue, and we collectively didn't see a much better way.

[ghost]

Simply reading cmi is not a safe approach. The fact that it uses functions from the typer rather than do the lookup manually is just an implementation detail.

The only way to reliably use feedback from the typer in ppx rewriter would be to limit the scope of ppx to extension point and attributes. Then the expansion could occur directly during typing where we have the correct typing environment.

[gasche]

But we still have the problem of what to do during the dependency-computation phase, where we don't yet have the typing environment at hand (at least given the current structure; something like codept integrated in the type-checker could solve that), but we still want to know if the plugin would add dependencies.

Relit has an approach to this chicken-and-egg problem using plugin-level metadata to register dependencies for plugin-generated code. If I remember correctly, it generates "fake" code in a first pass that is not semantically correct but results in those dependencies being reported to ocamldep; we could use the same approach here, per-plugin: eq would generate nothing but json would generate = Yojson.foo or something to add a dependency on its JSON library.

[ghost]

Well, you could generate fake code or you could simply have a specific API, i.e. a rewriter could register two entry points for a given extension point: a way to query dependencies and a way to expand the code.

These are details that one would sort out while designing the system. In any case, it is important to think about what features we want to provide to users and what is the scope of the system. This would ensure that ppx would provide a maybe more limited but more consistent feature set that are guaranteed to work well together. I feel like this is what is missing at the moment.

[gasche]

In the short term I wonder if there is something we (I?) should do for ppx_deriving as it exists. Hard-coding a special path for ocamldep sounds like the pragmatic approach, but it annoys me to have to audit each plugin.

[ghost]

You can always document it as a known bug. Alternatively, you can make ppx_import erase the [@@deriving attribute] when called from ocamldep. Not very pretty but at least you'll never have to deal with this issue again.

[gasche]

Yes, but it also leads to incorrect dependencies being computed whenever "no dependency" is not the right choice for a plugin. This may not be an issue as long as the corresponding dependency is on a third-party module (other code somewhere will depend on it so it will be linked in anyway), but in the world of big dune monorepos all the modules are local so we may see build failures in this situation.

[ghost]

The inter-library graph is roughly the same whether you are in a mono-repo or not. There is no fine dependencies between modules of different libraries. If you depend on a library, you just depend on a single thing that depend on all the modules of the library.

If we wanted finer dependencies for incremental compilation, I suppose we could change the compiler to report what files it actually read so that we could narrow the dependencies after running the compiler. And in this case that still wouldn't be an issue.