Add [@@@expand_inline] and support for floating attribute context free transformations

[jaymody]

Adds new context free transformations [@@@expand_inline <unexpanded-structure>] <expanded-structure> [@@@end] and [@@@expand_inline: <unexpanded-signature>] <expanded-signature> [@@@end].

For example:

module T = struct
  [@@@expand_inline let foo = [%ten]]
  [@@@end]
end

module type S = sig
  [@@@expand_inline val foo : [%str]]
  [@@@end]  
end

Would trigger the corrections (assuming we've registered ppxes to handle [%ten] and [%str] accordingly):

module T = struct
  [@@@expand_inline let foo = [%ten]]
+ let foo = 10
  [@@@end]
end

module type S = sig
  [@@@expand_inline val foo : [%str]]
+ val foo : string
  [@@@end]  
end

More generally, we provide users of ppxlib four new ways to create a Context_free.Rule.t:

  type ('item, 'parsed_payload) attr_floating_inline =
    ('item, 'parsed_payload) Attribute.Floating.t ->
    (ctxt:Expansion_context.Deriver.t -> 'parsed_payload -> 'item list) ->
    Context_free.Rule.t

  val attr_str_floating_expect : (structure_item, _)  attr_floating_inline
  val attr_sig_floating_expect : (signature_item, _)  attr_floating_inline
  val attr_str_floating_expect_and_expand : (structure_item, _)  attr_floating_inline
  val attr_sig_floating_expect_and_expand : (signature_item, _)  attr_floating_inline

For example:

  let my_attr =
    Ppxlib.Attribute.Floating.declare
      "my_ppx"
      Structure_item
      Ast_pattern.(pstr __)
      (fun x -> x)
      ...
  in
  let my_expand_fn ~ctxt payload =
    ...
  in
  let my_rule =
    Ppxlib.Context_free.Rule.attr_str_floating_expect
      my_attr
      my_expand_fn
      ~expand_items_inline:true
  in
  Driver.register_transformation "my_ppx_name" ~rules:[ my_rule ]

Will register a transformation that can be used as:

    [@@@my_ppx <some_payload>]
    <output_of_ppx_gets_inlined_via_ppx_corrected_file>
    [@@@end]

attr_str_floating_expect registers an expect transformation from a floating attributes in a structure item context. attr_str_floating_expect_and_expand is similar, but it also "expands" the output of the transformation by running it against other context free transformations. The sig versions of these functions behave similarly, but for floating attributes in signature item contexts.

Motivation

1. @@@expand_inline provides an alternative way to view ppx expanded code. Currently, the best way to do this would be to run your ppx.exe against an entire file and save that output to a separate file. This requires setting up a new build rule and switching files every time you want to see the output. Furthermore, there's no way to select which parts of a program you want to expand with file based expansion. @@@expand_inline does not have these limitations.
2. @@@expand_inline allows a library to use a ppx that depends on the library itself. For example:

• Compile library Foo
• Compile ppx.exe, which depends on Foo
• Compile Foo again, but this time we can add an expand_inline block where the payload can contain ppx code handled by ppx.exe. Since the expanded code gets inlined, and attributes are ignored by the compiler, we can repeat this cycle.

Reviewing

It's probably best to initially review this PR commit-by-commit:

• First commit (b110b6e): Adds the general attr_{str,sig}_floating_expect{,_and_expand} transformations.
• Second commit (600da3e): Adds expand_inline using attr_{str,sig}_floating_expect_and_expand.
• Third commit (ef156ee): Adds the ability to convert [@@ocaml.doc "foo"] comments to [@@ocaml.doc {|foo|}. This is useful if code inside the payload of an expand_inline contains doc comments. Without this, long doc comments will get formatted using the regular "" syntax, which makes the doc comment unreadable in the expanded code.

Considerations

• expand_inline doesn't expand anything if used alongside the -no-merge flag. This just means that users should probably not be using expand_inline (or any _and_expand transformation) alongside -no-merge.
• We should consider renaming @@@deriving.end to @@@ppxlib.inline.end.

[jaymody]

Seems like patdiff is not included as a dependency during CI, which is causing CI to fail.

[NathanReb]

patdiff dependency cone includes core which we don't want to have as a dependency even for testing as it makes it hard to support the whole range of compilers we want to support.

You can overwrite the default diff command with the -diff-cmd driver flag.

[NathanReb]

Out of curiosity, what's the use case for the simpler expect version compared to the expect_and_expand?

Do you have a concrete usecase for it already? It seems to go a bit against the context-free rules philosophy, where the result of any expansion rules is recursively expanded.

[NathanReb]

The code looks good to me, thanks!

This needs a changelog entry and it would be great to add a bit of documentation for the [@@@expand_inline ...] feature.

As I mentioned in the comments earlier, I'd rather avoid adding the expect with no expand versions unless strictly necessary.

[NathanReb]

Looking through our current handling of other expect expansion rules it seems that indeed other expect_items are NOT expanded further after their initial generation. I'm a bit surprised by this behaviour and that seems to be a bug rather than actual desired behaviour. I'll investigate!

[jaymody]

Looking through our current handling of other expect expansion rules it seems that indeed other expect_items are NOT expanded further after their initial generation. I'm a bit surprised by this behaviour and that seems to be a bug rather than actual desired behaviour. I'll investigate!

I think in some cases you don't want to expand on inlined code, for example maybe you have a deriver that itself outputs code that has [@@deriving] in it or some other ppx code:

type t = (int * string) * (char * string) [@@deriving_inline derive_compare_for_parts]

type p1 = int * string [@@deriving compare]
type p2 = char * string [@@deriving compare]

[@@@end]

In the above example, if you're intention is just to communicate the types, expanding the [@@deriving] would clutter up the code and make it hard to read. And even if you really do want to expand the inlined code, you could in an [@@@expand_inline].

[jaymody]

Out of curiosity, what's the use case for the simpler expect version compared to the expect_and_expand?

Do you have a concrete usecase for it already? It seems to go a bit against the context-free rules philosophy, where the result of any expansion rules is recursively expanded.

We don't have a concrete use case yet, but similar to my above comment, it may be useful to write a transformation where the output is not expanded:

[@@@make_variants "foo", "bar", "baz"]
type t = | Foo | Bar | Baz [@@deriving compare]
[@@@end]

However, I would be fine removing the _expect versions and just leaving the _expect_and_expand since the former feels very niche and I don't know if we'll ever have a good concrete use for it. I think @ceastlund had some thoughts regarding this.

[NathanReb]

The thing that puzzles me with this behaviour is that [@@deriving x] and [@@deriving_inline x] won't generate the same OCaml code. Furthermore, in your example:

type t = (int * string) * (char * string) [@@deriving_inline derive_compare_for_parts]

type p1 = int * string [@@deriving compare]
type p2 = char * string [@@deriving compare]

[@@@end]

Those [@@deriving compare] would never be expanded.

Anyway, I digress here as this is not related to this PR. I'd be in favor of keeping only the expect_and_expand variants as it will also simplify the code and I think having a single variant will be less confusing to users that would want to write their own ppx for expanding floating attributes.

[NathanReb]

Thanks for the documentation and for fixing the diff command in the tests!

Let's wait on @ceastlund take on the expect (without expand) variants to decide what to do with those!

[jaymody]

Those [@@deriving compare] would never be expanded.

Wouldn't it? When ppxlib generates the code the be inlined, yes those [@@deriving]'s are unexpanded, but once it actually gets accepted and inlined, and the user recompiles their code, it will get expanded (just, in this case, without them seeing the expansion, which is the point).

(in any case, I still think it would be okay to get rid of the expect without expand versions)

[NathanReb]

Yes good point, that's what I failed to see as I was only thinking in terms of a single ppxlib driver invocation.

Indeed if you promote the generated code, it will be expanded next time you compile it.

I guess I only thought about [@@deriving_inline ...] as a way to use ppx derivers without having to register them as a build dependency, hence why I had trouble understanding why one would want unexpanded code in the inlined items but I guess it's not the only usecase out there.

[ceastlund]

I don't have strong opinions on whether we include the non-_and_expand versions.