Add characterization tests for context-free rule application order

[NathanReb]

While working on #271 I sadly discovered that attribute based code generation is applied before expanding the node with the attributes attached.

It took me some time to go through the context-free rule application AST traversal and decided to add some tests both to convince myself that it worked as it was supposed to and to document the not so expected behaviour I discovered.

As a follow up to this and after talking about with @jeremiedimino, I'll try to change that behaviour and see how it impacts the opam universe. If the results are positive, I think it's worth changing that behaviour. In particular because it's a pretty big blocker for #271.

[pitag-ha]

Very nice new tests, thanks!

And of course for ppx_import it's unfortunate that what you've shown in the last test is the current behavior. A couple of thoughts about that:

I guess from an implementation perspective it makes sense that that's the behavior (even though of course it's not the wanted behavior): the attribute is on a higher level of the tree than the extension node, so when traversing the tree in a top down manner we hit the attribute first. Is that right?

And related to that: how far to do want to go applying extension nodes down the subtree before applying the deriving? For example, if we have a type declaration that's a little bit more complex, e.g.

type 'a t = [%id: int] constraint 'a = [%idc]
[@@deriving derived]

Do we only want to espand [%id: int] before the deriving or also [%idc]?

Also, I guess that the behavior you've shown is the same for module type declarations, isn't it? So here the same question: do we first want to rewrite all extension nodes belonging to the subtree starting at the module type declaration before coming to the deriving? I.e. possibly recursing down the whole Pmty_signature signature?

[NathanReb]

My intuition is that we should do it the other way around. Consider the following example:
I use two different ppx-es in my project, ppx-a which is an extension that expands to the inferred type of the payload
(e.g. [%a 0] expands to int), and ppx-deriving-b which derives some function, based on the type declaration it's attached to.
Those two ppx-es are developed separately by different people and are not meant to interact with each other in any predetermined way.

Say that for some reason I want to write the following code:

type t =
  { some_int : [%a 0]
  ; some_string_list : [%a ["s"; ""]]
  }
[@@deriving b]

This is likely to give me an error because ppx-deriving-b needs to know the type of the records fields but it doesn't know how [%a ...] is interpreted and therefore can't derive anything meaningful out of it.

If we swap the order, what ppx-deriving-b will receive as input to its expander function is the following type declaration:

type t =
  { some_int : int
  ; some_string_list : string list
  }

I can't think of any valid reason why in the general case we'd want to do it the other way around. I might be missing something here but my impression is that the only case where this is useful is if you want to pass information to the deriver that you can't express with the base OCaml language but there's already a solution here which is to use attributes. E.g. you could write something like:

type t =
  { some_int : int [@b.info 0]
  ; some_string_list : string [@b.info ["s"; ""]]
  }
 [@@deriving b]

My impression is that the current situation doesn't allow for something extra but instead restrict how you can use ppx-es together. It should really be a win win here.

I'll write the patch and test it against the opam universe to see if it breaks anything there but I'm pretty confident it won't.

You are also right that this is a strong blocker for ppx_import port, not only for the special type t = [%import: ...] syntax but also for the regular module type extension.

[NathanReb]

Do we only want to espand [%id: int] before the deriving or also [%idc]?

To answer that specific question, I think that we want to fully transform the node before passing it to the deriver.

[pitag-ha]

I think that we want to fully transform the node before passing it to the deriver.

Yeah, that sounds good.