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
-ppx could be made working in toplevel #5904
Comments
Comment author: @alainfrisch Commit 13278 (trunk) adds support for -ppx to the toplevel. Each phrase (not directives) is passed to external rewriters. Not that when the toplevel is used to execute a script (passed on its command-line, or with #use), then each phrase is rewritten independently. It might be worth sending the whole script to the rewriter instead. It would also allow the rewriter to rewrite directives. Actually, it would make sense to allow the rewriters to maintain their state across phrases. This could be done by letting the rewriters send (and read back) some extra custom data in addition to the ASTs; this requires the rewriter to maintain a clean notion of state, which can be easily serialized. Otherwise, we could keep the rewriters alive as external processes and talk with them for each phrase. But this requires more complex process control logic (from module Unix). |
Comment author: meyer That sounds awesome, thanks. |
Comment author: @gasche Is this issue resolved? |
Comment author: @alainfrisch Support for -ppx has been added to the toplevel, but it can be improved (see my note : "Actually, it would make sense to allow the rewriters to maintain their state across phrases..."). |
Comment author: @alainfrisch A #ppx directive has been added, to add dynamically extra ppx rewriters during a toplevel session. |
Comment author: @alainfrisch Commit 15314 adds a notion of "persistent cookies", which can be set by ppx processors and retrieved on further invocations when called from the toplevel. It is the responsibility of each ppx processor to store a cookie that represent its state and then restore it (using Ast_mapper.get_cookie/set_cookie). Note that cookies are available when the function registered through Ast_mapper.register is executed, so it is possible to set some global variables there. |
Comment author: @gasche Sorry for reacting a bit late, but the current API for ppx (** {2 Helper functions to call external mappers} *) val add_ppx_context_str: tool_name:string -> Parsetree.structure -> Parsetree.structure val add_ppx_context_sig: tool_name:string -> Parsetree.signature -> Parsetree.signature val drop_ppx_context_str: restore:bool -> Parsetree.structure -> Parsetree.structure val drop_ppx_context_sig: restore:bool -> Parsetree.signature -> Parsetree.signature (** {2 Cookies} *) (** Cookies are used to pass information from a ppx processor to val set_cookie: string -> Parsetree.expression -> unit I find this API quite indirect. Here is what I understand it does:
Aspects (1) and (2) are rather orthogonal, but the API only provide I would rather suggest an interface along the following lines:
Similarly, I would be interested in an immutable type for cookies with As a concrete example of use for such immutable data structures, |
Comment author: @alainfrisch Hi Gabriel, The API for cookies (set/get_cookie) is intended to be used by ppx rewriters. The context function are "less public", and currently mostly intended to be used by the compiler itself or closely related tools. But don't hesitate to send a patch with a better API! |
git-svn-id: http://caml.inria.fr/svn/ocaml/version/4.02@15411 f963ae5c-01c2-4b8c-9fe0-0dff7051ff02
…herry-picked from trunk, commit 15414.) git-svn-id: http://caml.inria.fr/svn/ocaml/version/4.02@15415 f963ae5c-01c2-4b8c-9fe0-0dff7051ff02
Original bug ID: 5904
Reporter: meyer
Assigned to: @alainfrisch
Status: closed (set by @xavierleroy on 2016-12-07T10:36:48Z)
Resolution: fixed
Priority: low
Severity: feature
Fixed in version: 4.02.1+dev
Category: ~DO NOT USE (was: OCaml general)
Monitored by: @hcarty
Bug description
I'd like to test my -ppx code in toplevel. Currently -ppx works just offline, using the file system.
This is a feature wish, and probably requiring some effort, nevertheless I think it's worthwhile.
The text was updated successfully, but these errors were encountered: