Drop items without source text in Code_matcher

[nmatschke]

Code_matcher assumes that the items following an inline block's anchor are
exactly the items that were parsed from the source file, and rewrites their source text
in place. That assumption breaks when the anchor is itself inside code generated by
another ppx. In particular, ppx_template wraps template-mangled definitions in (**/**)
markers so odoc hides them, and those markers inherit the location of the node they were
generated from. So

[%%template:
type t = int [@@kind.explicit k = (value, value_or_null)] [@@deriving_inline globalize]

[@@@end]]

expands to a (**/**) item whose location is the whole type declaration, and the
corrected code replaces the declaration with the derived signature instead of inserting
it after.

Fix this by dropping items that start before the end of the anchor item, since those
cannot correspond to source text following it.

[NathanReb]

The change looks sane to me but I think it is worth a test!

[NathanReb]

Could you also rebase it on top of main to fix the 5.5 builds in the CI, sign-off your commits with -s and add a changelog entry please?

[NathanReb]

This sounds a little bit odd, I think adding the ppx_template example here would make a bit more sense as otherwise, ppx generated deriving_inline nodes sounds a bit crazy and something we don't necessarily want to support.

[NathanReb]

Can you elaborate a bit more on ppx_template and explain what it does and why a ppx would generate deriving_inline nodes?

[nmatschke]

Sure, happy to expand a bit.

ppx_template is a ppx that expands one type/function/module/etc. declaration/definition in the source code to multiple declarations/definitions in the ppx-generated code, with mangled names. This isn't useful for vanilla OCaml, but is really common in OxCaml because the compiler doesn't (yet) natively know how to generate code which is polymorphic over some of the language extensions.

Here's an example:

[@@@expand_inline:
  type%template ('a : k) ref = { mutable contents : 'a } [@@kind k = (value, bits64)]]

type ('a : value) ref = { mutable contents : 'a }
and ('a : bits64) ref__bits64 = { mutable contents : 'a }

[@@@end]

In practice, we found that users would commonly generate so much code that odoc-generated documentation was functionally unusable. So, we modified ppx_template to hide generated code from odoc if all of the identifiers were mangled, using the special (**/**) doc comment form:

[@@@expand_inline:
  type%template ('a : k) ref = { mutable contents : 'a } [@@kind k = (float64, bits64)]]

[@@@ocaml.text "/*"]

type ('a : float64) ref__float64 = { mutable contents : 'a }
and ('a : bits64) ref__bits64 = { mutable contents : 'a }

[@@@ocaml.text "/*"]

[@@@end]

Ok, now, the problem is I have another [@@deriving]-based ppx which I want to test by expanding the generated code, and I specifically want to test its behavior in the presence of ppx_template. I could do this one of two ways.

The first is that I could use [@@@expand_inline]:

[@@@expand_inline:
  type%template ('a : k) ref = { mutable contents : 'a }
  [@@deriving sexp_of] [@@kind k = (float64, bits64)]]

[@@@ocaml.text "/*"]

type ('a : float64) ref__float64 = { mutable contents : 'a } [@@deriving sexp_of]
and ('a : bits64) ref__bits64 = { mutable contents : 'a }

include sig
  [@@@ocaml.warning "-32"]

  val sexp_of_ref__float64
    : ('a : float64).
    (('a : float64) -> Sexplib0.Sexp.t) -> ('a : float64) ref__float64 -> Sexplib0.Sexp.t

  val sexp_of_ref__bits64
    : ('a : bits64).
    (('a : bits64) -> Sexplib0.Sexp.t) -> ('a : bits64) ref__bits64 -> Sexplib0.Sexp.t
end
[@@ocaml.doc "@inline"] [@@merlin.hide]

[@@@ocaml.text "/*"]

[@@@end]

But notice that there's still a [@@deriving] in the output - the compiler is actually ingesting two copies of the derived code, one from the [@@deriving], and one from the inlined code. In fact, many derivers, including the one I want to test, generate code that itself contains more types with more derivers, and this can easily break [@@@expand_inline], because we end up with two copies of the code, causing shadowing issues et cetera. To fix this, I think [@@@expand_inline] would have to recursively strip [@@deriving] attributes, along with any attributes the associated derivers would consume, since otherwise we end up with unused attributes in the source code. This is arguably a more "correct" change in some sense, but it's heftier.

The second option is to use [@@deriving_inline]:

[%%template:
type ('a : k) ref = { mutable contents : 'a }
[@@deriving_inline sexp_of] [@@kind k = (float64, bits64)]

include sig
  [@@@ocaml.warning "-32"]

  val sexp_of_ref__float64
    : ('a : float64).
    (('a : float64) -> Sexplib0.Sexp.t) -> ('a : float64) ref__float64 -> Sexplib0.Sexp.t

  val sexp_of_ref__bits64
    : ('a : bits64).
    (('a : bits64) -> Sexplib0.Sexp.t) -> ('a : bits64) ref__bits64 -> Sexplib0.Sexp.t
end
[@@ocaml.doc "@inline"]

[@@@end]]

This works great, except the Code_matcher sees the [@@@ocaml.text "/*"] in ppx_template's output, with a source location inherited from the Psig_type, and decides to delete starting from that location rather than from the [@@deriving_inline]-generated code.

So, this PR modifies Code_matcher to drop any items whose source location is before the end of the [@@deriving_inline], because these obviously can't have been present in the real source text.

[nmatschke]

ppx_template, in general, passes through any source code in its payload unchanged, except for things annotated with specific attributes it understands, like [@kind], [@mode], and so on. So, if its payload already contains a [@@deriving_inline], it will just forward that on in its output, hence "ppx generated deriving_inline nodes".

Note that this form doesn't work:

type%template ('a : k) ref = { mutable contents : 'a }
[@@deriving_inline sexp_of] [@@kind k = (float64, bits64)]

[@@@end]

Because it's equivalent to:

[%%template:
type ('a : k) ref = { mutable contents : 'a }
[@@deriving_inline sexp_of] [@@kind k = (float64, bits64)]]

[@@@end]

That is, the [@@@end] is outside the [%%template: ], yielding ppxlib: [@@@ppxlib.inline.end] attribute missing.