Interest in effects to replace ~loc

[N1ark]

Hi! I started using ppxlib quite heavily recently, and it's super useful, thanks! However, I find that having to pass a ~loc to all constructors / having to always have a loc variable in scope is a bit tedious, and sometimes leads to overly verbose code.

In my ppx deriver I defined a tiny wrapper around all constructors that take in a ~loc, and defined a simple algebraic effect to instead store the location without having to make it explicit. The implementation is just this:

open Ppxlib
open Ast_builder.Default

type _ Effect.t += Get_loc : location Effect.t

let with_loc (loc : location) f =
  let open Effect.Deep in
  try f () with effect Get_loc, k -> continue k loc

let get_loc () = Effect.perform Get_loc

let ( let@ ) = ( @@ )

(* Override anything we need *)
let pexp_ident x = pexp_ident ~loc:(get_loc ()) x
let ppat_any () = ppat_any ~loc:(get_loc ())
(* etc ... *)

(* Usage is simple! *)
let@ () = with_loc my_loc in
let my_ident = pexp_ident (Lident "foo") in ...

This allows me to write the ppx without having to worry about locations: I just wrap the top-most element with the location i care about, and optionally re-wrap sub-items if i want a more precise location there. I still need a loc in scope when using the ppx, but that just requires a let loc = get_loc ()

Is this something that would be of interest to other users of ppxlib? I would be happy to open a PR and add this as an optional replacement to Ast_builder.Default. Or does this go completely against the intended use of locations? At least in our codebase this has proven quite useful :)

[NathanReb]

Ast_builder provides a functor you can use so that you don't have to pass a location explicitly everytime:

module Located_builder = Ast_builder.Make(struct let loc = ... end)

I think this should work well enough for the vast majority of cases even though it feels a bit heavier than your solution. I see you've been asking on OCaml's Zulip about functor application performances, was it related to this? What were the conclusions? What's the performance hit with the effect based solution?

All this being said though, using effects would make us incompatible with older compilers so that's not acceptable for us at the moment, despite your solutions elegance.

[N1ark]

using this functor would however lose precision if i want to use several locations while generating my AST :/ really what I need is Ast_builder.Make' where the input module has a val loc : () -> location instead, rather than a static one.

i understand you can't use it here for retro-compatibility, adding an Ast_builder functor that would enable this as described would be of great help if that's something you'd find of use :) happy to open a PR

about Zulip yeah I was asking there about it but for something unrelated, I haven't tried for this use case but I'm pretty happy with the effect solution so i'll stay with that

[NathanReb]

I understand! You could eventually apply the functor locally a few times and pass the module as an argument but I can see how that's not as satisfying as your current solution.

I do like the idea of providing a with_loc kind of interface as an alternative to the existing modules, it does not necessarily has to go through a functor and could provide the with_loc wrapper itself:

let@ () = Ast_builder.Context.with_loc loc in
let my_ident = Ast_builder.Context.pexp_ident (Lident "foo") in ...