ppxlib icon indicating copy to clipboard operation
ppxlib copied to clipboard

Allow giving priorities to other PPXs in some sub-expressions via callbacks

Open kit-ty-kate opened this issue 4 years ago • 1 comments

Let's say I have two PPXs written with ppxlib, that can be used separately or in conjunction with one another as follow:

if%ppx1 [%ppx2] then ...

ppx1 does parse what is given in the condition, however in my example ppx1 is always executed first and thus fail because it cannot parse the other extension point.

Currently both PPXs are written more or less the same way:

let traverse = object
  inherit Ppxlib.Ast_traverse.map as super

  method! expression = function
    | { pexp_desc = Pexp_extension ({txt = "ppx1"; _}, pstr); _ } ->
        begin match pstr with
        | PStr [{pstr_desc = Pstr_eval ({pexp_desc = Pexp_ifthenelse (cond, _, _); _ }, _); _ }] ->
          (* Parse "cond" here, then do something else *)
        | _ -> ...
        end
      | expr -> super#expression expr
  end

let () = Ppxlib.Driver.register_transformation ~impl:traverse#structure "ppx1"

In this example for ppx1, I'd like to be able to say something like:

        | PStr [{pstr_desc = Pstr_eval ({pexp_desc = Pexp_ifthenelse (cond, _, _); _ }, _); _ }] ->
          (* Let other PPX rewriters deal with this expression first *)
          let cond = super#expression cond in
          (* Parse "cond" here, then do something else *)

However super#expression does not call the other PPX rewriters as I first thought. I feel like something like this could be useful for other PPXs as well.

Note: In my case ppx2 is simple enough to be rewritten using Context_free.Rule so it can be handled before ppx1 but I suppose there could be combination of PPXs that would not be able to do that.

kit-ty-kate avatar Aug 12 '20 12:08 kit-ty-kate

I totally agree there is a use case for this, in particular we considered adding support for this through a ppxlib callback provided to one's expand function when we considered combining ppx_view and metaquot.

For what it's worth I think that if we add support for this, it will only work for context-free transformations anyway as these are the only ones with a clear composition semantic!

As a quick side note, in this particular case, unless it's strictly necessary I think ppx1 should be defined using Extension.declare rather than as a whole AST transformation, especially if you want to be able to combine it in such a way! You can take a look to the brand new Getting started section of the manual if you need pointers on how to do that!

NathanReb avatar Aug 12 '20 15:08 NathanReb