ocaml / dune

A composable build system for OCaml.
https://dune.build/
MIT License
1.6k stars 395 forks source link

Interaction between nested ppx_rewriters and cookies #3426

Open jberdine opened 4 years ago

jberdine commented 4 years ago

Actual Behavior

If I have a ppx_rewriter such as:

(library
 (public_name ppx_trace)
 (kind
  (ppx_rewriter
   (cookies
    (ppx_trace_enabled %{env:PPX_TRACE_ENABLED=0}))))
 (preprocess no_preprocessing)
 (libraries ppxlib ppx_trace.trace))

Then I can invoke it using (preprocess (pps ppx_trace)) in e.g. library stanzas.

But if I wrap it into another rewriter, such as:

(library
 (name ppx_combined)
 (kind ppx_rewriter)
 (libraries ppx_compare ppx_trace)
 (preprocess no_preprocessing))

Then using (preprocess (pps ppx_combined)) in other stanzas performs the rewriting, but does not pass the cookie.

(FWIW, the purpose of the ppx_combined rewriter is just to deduplicate the list of rewriters that appears numerous times. Maybe there is an easier/better way to name a list of rewriters?)

Expected Behavior

I did not expect that wrapping ppx_trace in ppx_combined would influence the cookie passing specified in the stanza defining the ppx_trace rewriter.

I can try to boil down a repro, but wanted to ask if this is actually intended behavior before spending the time.

ghost commented 4 years ago

This indeed looks like a bug. Combining ppx rewriters this way is indeed the intended way to name sets of ppx rewriters. That's way ppx_jane does for instance

rgrinberg commented 3 years ago

But if I wrap it into another rewriter, such as:

Is this still allowed? I thought we only allow to pass rewriters in preprocess now.

jberdine commented 1 year ago

In case it is useful when looking at old issues, this still is an issue with dune 3.8.0.