ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.62k stars 401 forks source link

[RFC] Subcommand to show result of ppx expansion #4470

Closed smorimoto closed 2 years ago

smorimoto commented 3 years ago

I don't know if anyone still remembers that I made a suggestion about this in a dev chat before, but I'd like to suggest dune expand, a subcommand to show result of ppx expansion. For now, we need to call up a lot of relevant tools like ocamlmerlin to check them like this: https://gist.github.com/Khady/e7b197e60a5065651815bc7b19c07a24 But if we can check them with this subcommand, it's very useful.

bobot commented 3 years ago

I think it is a very good idea, just wondering:

smorimoto commented 3 years ago

other possible name: dune ocaml expand, or dune ocaml expand-ppx

I don't have a strong opinion about subcommand name at all. The name is a tentative name that @CraigFe and I used when we were talking about the cargo expand.

Should the file be part of a library or executable? It seems since otherwise we don't know which file to call.

Yes, it should be part of a project that is managed by dune.

ghost commented 3 years ago

Regarding the name, dune expand seems fine to me. We want to put things that are OCaml specific under dune ocaml, such as utop or merlin. But the notion of preprocessing a source file seems general enough and could be mapped in most languages. If the command was specific to the inner working of ppx rewriters, then it should indeed go under dune ocaml.

@smorimoto are you interested in writing a PR for this feature? We can give you some pointers.

jberdine commented 3 years ago

I would also find this to be useful. Now and again I hunt through _build/log to find an interesting invocation of ppx.exe, remove the dump-ast flag, and rerun it to see the preprocessed output.

I wonder whether people generally think of this as "expansion". My first thought would be to call it "preprocessing", and use dune preprocess.

ghost commented 3 years ago

preprocess does seem more precise. expand could mean other things, such as "expand variables": dune expand %{cmo:...}

smorimoto commented 3 years ago

Indeed, I love that name.

@jeremiedimino Yes, I would like to work on this.

ghost commented 3 years ago

Alright, so Dune already stores the output of preprocessing to disk. So all you need to do here is:

  1. figure out the name of the preprocessed file for a given source file
  2. ask Dune to build it
  3. pretty-print this file

For the general structure of the dune preprocess command, you can start by copy, pasting and adapting bin/utop.ml, which has a similar structure (it figures out the name of the custom utop binary, ask Dune to build it and then executes this binary). You will then need to add the command value defined in your new module to the all list in bin/main.ml.

There are different ways to do 1. To keep things simple for now, I suggest that we rely on the the assumption that the preprocessed filename will be named file.pp.ml. You can find the code inserting the .pp suffix in the pped function in src/dune_rules/module.ml.

Once you have the .pp filename, 2 should be straightforward by passing this filename to Build_system.build.

For 3, the .pp file will often contain a marshaled OCaml AST that Dune doesn't know how to read. So we need to use a separate tool to print it. We can use the OCaml compiler itself:

$ ocamlc -stop-after parsing -dsource <file.pp.ml>
smorimoto commented 3 years ago

Thank you for the very detailed pointer! Let me tackle it!

rgrinberg commented 3 years ago

@smorimoto I think this is now directly supported by the editor through the ast explorer feature.

jberdine commented 3 years ago

@rgrinberg Do you have a link? Maybe I have not been paying attention but I don't know which "editor" or "ast exporter" you are referring to.

rgrinberg commented 3 years ago

What I had in mind was the new AST exploration feature in the vscode plugin: https://github.com/ocamllabs/vscode-ocaml-platform/pull/666

It's an editor specific feature so I suppose I'll re-open this. But the way to support this is to open add an appropriate request to RPC and expose it via the lsp server to the editor.

kiranandcode commented 2 years ago

Might be relevant: https://gitlab.com/gopiandcode/dune-expand

cannorin commented 2 years ago

Implementation started in https://github.com/ocaml/dune/pull/5615.

anmonteiro commented 2 years ago

This feature doesn't seem to work on reason files.

Minimal repro:

;; dune
(executable
 (name x)
 (preprocess
  (pps ppx_deriving.std ppx_enumerate)))
// x.re
[@deriving (enumerate, show)]
type x = [ | `lol | `other];
$ dune describe pp ./x.re
[@deriving (enumerate, show)]
type x = [ | `lol | `other];
EduardoRFS commented 2 years ago

A temporary workaround is

dune describe pp ./x.re.ml
Lupus commented 2 years ago

Thanks for the workaround @EduardoRFS ! Would be nice to have this working for Reason natively...

ejgallego commented 1 year ago

Hi folks, this feature seems broken, for example in https://github.com/ejgallego/coq-serapi I do:

dune describe pp serlib/ser_int.mli

and it doesn't work.

Maybe it is because of this dune config serapi uses:

 (preprocess (staged_pps ppx_import ppx_sexp_conv ppx_hash ppx_compare ppx_deriving_yojson))

Am I doing something wrong, or should I open an issue?

Alizter commented 1 year ago

@ejgallego Does the workaround above work? i.e

dune describe pp serlib/ser_int.mli.ml
ejgallego commented 1 year ago

Nope, nothing I tried works.

Alizter commented 1 year ago

@ejgallego Please open a bug report then.