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

Unable to use future_syntax with dune 1.10.0 #2262

Closed
Dean177 opened this issue Jun 8, 2019 · 4 comments
Closed

Unable to use future_syntax with dune 1.10.0 #2262

Dean177 opened this issue Jun 8, 2019 · 4 comments

Comments

@Dean177
Copy link
Contributor

Dean177 commented Jun 8, 2019

I am attempting to use the new ocaml-syntax-shims with other preprocessors on dune 1.10.0

but get the following error:

 (pps ppx_inline_test -- -pp %{bin:ocaml-syntax-shims})
                                         ^^^^^^^^^^^^^^^^^^^^^^^
Error: %{bin:..} isn't allowed in this position

Reproducing example:

(library
  (name future_example)  
  (inline_tests)
  (preprocess (pps ppx_inline_test -- -pp %{bin:ocaml-syntax-shims})))
let (let+) l f = 
  List.map f l

let add_one l = 
  let+ elem = l in
  elem + 1

let%test _ =
  add_one [1;2;3] = [2;3;4]

See also #1934

@aalekseyev
Copy link
Collaborator

Apparently we don't expand %{bin} here, probably for no particular reason.

As a workaround you could try something like this:

(library
 (name future_example)
 (preprocess (pps ppx_inline_test -- -pp ./ocaml-syntax-shims))
 (preprocessor_deps ./ocaml-syntax-shims)
 )
(rule (copy %{bin:ocaml-syntax-shims} ./ocaml-syntax-shims))

@mlasson, I'm told you know something about ppx flag expansion.
Do you think it would make sense to support more variables in ppx flag expansion?

@Dean177
Copy link
Contributor Author

Dean177 commented Jun 10, 2019

Thanks @aalekseyev, that works perfectly

@mlasson
Copy link
Collaborator

mlasson commented Jun 24, 2019

@aalekseyev : Yes it would totally makes sense, but I think some thinking should be put into that.

Currently, most macro are allowed only in actions (except for %{env} which feels like a hack).
Moreover, these expansions in flags are "static": they are triggered even if you don't build a thing and %{bin} could fail (because the binary is not in the installed world).

If we wanted to support more macros in preprocessors flags we would need to change the logic in order to delay the expansion at the build stage. Moreover, for consistency, we would probably want to do that for other kinds of flags. So this does not seem trivial to me (but I'm far from being an expert).

Finally, if we want to go in that direction (and why not ?), we may want to support other macros that may add dependencies to the build (so we will need not to forget to collect them and record them).

@emillon
Copy link
Collaborator

emillon commented Oct 11, 2022

I'm not sure about the timeline here, but this has been fixed in #2076 (dune 1.10). Using (lang dune 1.10) enables this.

@emillon emillon closed this as completed Oct 11, 2022
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