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

#require doesn't work in toplevel #30

Open
yminsky opened this issue May 4, 2020 · 9 comments
Open

#require doesn't work in toplevel #30

yminsky opened this issue May 4, 2020 · 9 comments
Labels
forwarded-to-js-devs This report has been forwarded to Jane Street's internal review system. not-clear-what-next-step-is This report is kept open as a reminder, but it is not clear what should happen next.

Comments

@yminsky
Copy link

yminsky commented May 4, 2020

I can enable ppx_sexp_value if I #require "ppx_jane", but not if I #require "ppx_sexp_value".

utop # #require "ppx_jane";;
─( 19:25:31 )─< command 1 >─────────────────────────────────────────────────────────────{ counter: 0 }─
utop # open Base;;
─( 19:25:36 )─< command 2 >─────────────────────────────────────────────────────────────{ counter: 0 }─
utop # [%sexp_of: int];;
- : int -> Sexp.t = <fun>
utop # #require "ppx_sexp_value";;
─( 19:23:52 )─< command 1 >─────────────────────────────────────────────────────────────{ counter: 0 }─
utop # [%sexp_of: int];;
Line 1, characters 2-9:
Error: Uninterpreted extension 'sexp_of'.

Any idea what's going on?

@ghost
Copy link

ghost commented May 18, 2020

This is expected. ppx_sexp_conv being a dependency is:

  • a public fact of ppx_jane
  • an implementation detail of ppx_sexp_value

We then carefully crafted the dependencies between these libraries to reflect this fact. So ppx_sexp_value not providing [%sexp_of: ...] is voluntary. You need to explicitly depend on ppx_sexp_conv to get access to it.

@ghost ghost closed this as completed May 18, 2020
@yminsky
Copy link
Author

yminsky commented May 18, 2020

Hmm. I can't figure out how to enable [%sexp_of] or [@@deriving sexp] in utop except by #require "ppx_jane". How should I do it?

@yminsky yminsky reopened this May 18, 2020
@ghost
Copy link

ghost commented May 18, 2020

It should work with: #require "ppx_sexp_conv"

@yminsky
Copy link
Author

yminsky commented May 18, 2020

It doesn't seem to work for me:

utop # #require "base";;
─( 12:49:49 )─< command 1 >───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────{ counter: 0 }─
utop # #require "ppx_sexp_conv";;
No such package: ppx_deriving - required by `ppx_sexp_conv'

@ghost
Copy link

ghost commented May 19, 2020

Ah, that's a deeper issue... For historical reasons, the toplevel relies on an older and more complicated way of composing and running ppx rewriters. Currently, you have to install the ppx_deriving package in order to use [@@deriving] plugins in the toplevel.

The reason it works with ppx_jane is a bit long to explain.

/cc @emillon, we should really look into changing this

@yminsky
Copy link
Author

yminsky commented May 19, 2020

Is a reasonable short-term hack to make ppx_deriving a dependency of the various ppxs? That way, when you install a PPX, you'll be able to actually use it in the toplevel without installing anything else.

@ghost
Copy link

ghost commented May 19, 2020

Such changes never have a 0 cost as everybody cares about dependencies and we have been in this situations for years. So I'm more inclined to keep it as a know quirk until we switch to something more straightforward.

@yminsky
Copy link
Author

yminsky commented May 19, 2020 via email

@emillon
Copy link

emillon commented May 25, 2020

Ah, that's a deeper issue... For historical reasons, the toplevel relies on an older and more complicated way of composing and running ppx rewriters. Currently, you have to install the ppx_deriving package in order to use [@@deriving] plugins in the toplevel.

The reason it works with ppx_jane is a bit long to explain.

/cc @emillon, we should really look into changing this

Yeah, curious to hear why it works with ppx_jane. I suppose it's related to the fact that ppx_jane is installed as a precompiled driver usable with #ppx as described in the README.

@aalekseyev aalekseyev added the not-clear-what-next-step-is This report is kept open as a reminder, but it is not clear what should happen next. label Sep 3, 2020
@github-iron github-iron added the forwarded-to-js-devs This report has been forwarded to Jane Street's internal review system. label Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
forwarded-to-js-devs This report has been forwarded to Jane Street's internal review system. not-clear-what-next-step-is This report is kept open as a reminder, but it is not clear what should happen next.
Projects
None yet
Development

No branches or pull requests

4 participants