Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[@@deriving_inline] requires deriver to be present during non-lint build #342

Open
sim642 opened this issue Jun 2, 2022 · 5 comments
Open

Comments

@sim642
Copy link
Contributor

sim642 commented Jun 2, 2022

The documentation states the following about [@@deriving_inline]:

You might notice that the resulting file when using
``[@@deriving_inline]`` needs no special treatment to be compiled. In
particular, you can build it without the ppx rewriter or even
ppxlib. You only need them while developing the project, in order to
automatically produce the generated code but that's it. End users of
your project do not need to install ppxlib and other ppx rewriters
themselves.

However, this appears not to be true, as I observed by doing the following:

  1. Added [@@deriving_inline yaml][@@@end] after a type declaration.
  2. Added (lint (pps ppx_deriving_yaml)) into the dune file.
  3. Ran dune build @lint --auto-promote, which did promote the inline implementations.
  4. Ran dune build, which fails with the following:
    10 |   [@@deriving_inline yaml]
                              ^^^^
    Error: Ppxlib.Deriving: 'yaml' is not a supported type deriving generator
    

It appears to me that ppxlib is still trying to run the deriver for [@@deriving_inline] when there's already an implementation present (which I guess makes sense for round-trip checking, #338), but does so even when that deriver was only active for linting and not the normal build.
This defeats the claim of being able to drop dependencies for ppx-s, since for the normal build to succeed, I still need that deriver also in dune's (preprocess (pps ...)).

Or maybe I'm trying to use [@@deriving_inline] in an unusual way: to drop some deriver dependencies, but not all (so still have others present). The motivation for doing that is to use an unreleased version of the deriver (using opam pin) in a package that is to be released on opam (where it cannot have pins) without being forced to wait for the ppx's release.

@ceastlund
Copy link
Collaborator

I think @@deriving_inline only "needs no special treatment" when ppxlib is not run at all. Once you start preprocessing, ppxlib is going to want to know how to expand that inline code.

It's possible there should be a flag to tell ppxlib to only bother with @@deriving_inline during linting, to allow this kind of piecemeal use of it. But it sounds like your use case is pretty niche, so I'm not sure how much this is likely to be prioritized.

A pull request adding some straightforward flag like that would be welcome, certainly.

@mbarbin
Copy link
Contributor

mbarbin commented Oct 18, 2024

I wanted to comment on this issue because I found myself with a use case quite similar to what is described by @sim642 here (contemplating how to remove compile time dependency to ppx-es in some projects, outside of dev-mode).

If @@deriving_inline cannot be used to limit your ppx dependencies that way, what is it currently used for? (I'm curious).

If work in this area is still welcome, I'd be interested in learning more. I think I could consider allocating time on this, perhaps contributing to the manual, and/or try adding support for extending what deriving_inline can be used for.

@mbarbin
Copy link
Contributor

mbarbin commented Oct 21, 2024

I published a concrete example of what I'm trying to do here.

Edit: In particular the diff in this commit shows the error triggered if I add ppx_compare to the linters but not to the preprocessors list, and then try to use deriving_inline.

@mbarbin
Copy link
Contributor

mbarbin commented Nov 26, 2024

@NathanReb thanks a lot for looking into #338 I'm highlighting this issue as well, as I believe this may be helpful additional context. To quote a previous comment from this conversation:

It's possible there should be a flag to tell ppxlib to only bother with @@deriving_inline during linting, to allow this kind of piecemeal use of it. But it sounds like your use case is pretty niche, so I'm not sure how much this is likely to be prioritized.

A pull request adding some straightforward flag like that would be welcome, certainly.

Perhaps that would be part of the puzzle if we think the use case of using deriving_inline to limit a project's dependencies, while using other ppx at the same time, is reasonable.

Here I am sharing why I'd fine this useful:

  1. I'd probably be more willing to onboard newer / less stable ppx if I can use them via deriving_inline constructs, and monitor their output.
  2. There's a also some ppx that do enforce a runtime-lib dependency, but in many case don't use it, or perhaps just a few functions. By using the ppx as a lint step only, you can also only add the runtime function required in some ad-hoc way, and remove the runtime dep entirely. I have some draft of using ppx like ppx_compare or ppx_equal that way, and avoiding the base runtime dep that comes transitively with ppx_compare.runtime-lib in certain projects that cannot afford to link base but could use of some ppx-help for defining equal and compare functions.
  3. Allowing the use of deriving_inline + constructs such as [%sexp _]. There is no _inline equivalent for the latter, so even if you are willing to switch all your ppx to _inline in an attempt to remove them from the build, you would still encounter the current limitation as you'd have to enable ppxlib during the build for the [%...] extensions (so this isn't a viable work around).

@NathanReb
Copy link
Collaborator

I think a flag for the driver would be a good start here. I'll get up to date with how dune invokes the driver when linting to see what we can do here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants