Skip to content

Conversation

@pitag-ha
Copy link
Member

@pitag-ha pitag-ha commented Dec 16, 2020

Before, the standalone_run_as_ppx_rewriter implementation was based on compiler-lib's Ast_mapper. With this commit, it reuses the adequate parts of the internal implementation of the main standalone. That has three advantages:

  • it takes away a burden for Astlib
  • it makes it possible to access the source file name from within the driver module
  • it makes it possible to implement that ppxlib preserves the compiler version, also when using standalone_run_as_ppx_rewriter or run_as_ppx_rewriter.

This commit also fixes the following: now, standalone_run_as_ppx_rewriter takes into account the loc-filename argument when provided. Since that argument forms part of shared_args, that's the expected behaviour.

When comparing the new implementation with the old one, note that in the new one the ppx context gets restored through extract_cookies and the cookies get updated through Intf_or_impl.to_ast_io with ~add_ppx_context:true.

@pitag-ha pitag-ha requested a review from xclerc as a code owner December 16, 2020 19:03
@pitag-ha pitag-ha force-pushed the standalone-run-as-ppx-rewriter branch 2 times, most recently from dc9c3fc to 96d33da Compare December 18, 2020 11:26
@pitag-ha
Copy link
Member Author

The ocaml-ci is green apart from the old compiler version and the arm32 jobs. Those failures aren't related to this PR.

Copy link
Collaborator

@NathanReb NathanReb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thanks for the great cram tests!

@pitag-ha pitag-ha force-pushed the standalone-run-as-ppx-rewriter branch 3 times, most recently from 71bc811 to e2c2008 Compare December 18, 2020 17:02
@pitag-ha pitag-ha force-pushed the standalone-run-as-ppx-rewriter branch from e2c2008 to a1d441d Compare December 28, 2020 11:15
Before, the standalone_run_as_ppx_rewriter implementation was based on
compiler-lib's Ast_mapper. With this commit, it reuses the adequate parts
of the internal implementation of the main standalone. That has two
advantages:
- it takes away a burden for Astlib
- it makes it possible to access the source file name from within the
driver module

This commit also fixes the following: now, standalone_run_as_ppx_rewriter
takes into account the `loc-filename` argument when provided. Since that
argument forms part of `shared_args`, that's the expected behaviour.

Signed-off-by: Sonja Heinze <[email protected]>
@pitag-ha pitag-ha merged commit cd0bf9f into ocaml-ppx:master Dec 28, 2020
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Jan 20, 2021
CHANGES:

- Driver (important for bucklescript): handling binary AST's, accept any
  supported version as input; preserve that version (ocaml-ppx/ppxlib#205, @pitag-ha)

- `-as-ppx`: take into account the `-loc-filename` argument (ocaml-ppx/ppxlib#197, @pitag-ha)

- Add input name to expansion context (ocaml-ppx/ppxlib#202, @pitag-ha)

- Add Driver.V2: give access to expansion context in whole file transformation
  callbacks of `register_transformation` (ocaml-ppx/ppxlib#202, @pitag-ha)

- Driver: take `-cookie` argument into account, also when the input is a
  binary AST (@pitag-ha, ocaml-ppx/ppxlib#209)

- `run_as_ppx_rewriter`: take into account the arguments
  `-loc-filename`, `apply` and `dont-apply` (ocaml-ppx/ppxlib#205, @pitag-ha)

- Location.Error: add functions `raise` and `update_loc`
  (ocaml-ppx/ppxlib#205, @pitag-ha)
NathanReb added a commit to NathanReb/opam-repository that referenced this pull request Jan 22, 2021
CHANGES:

- Fix ppxlib.traverse declaration and make it a deriver and not a rewriter
  (ocaml-ppx/ppxlib#213, @NathanReb)
- Driver (important for bucklescript): handling binary AST's, accept any
  supported version as input; preserve that version (ocaml-ppx/ppxlib#205, @pitag-ha)

- `-as-ppx`: take into account the `-loc-filename` argument (ocaml-ppx/ppxlib#197, @pitag-ha)

- Add input name to expansion context (ocaml-ppx/ppxlib#202, @pitag-ha)

- Add Driver.V2: give access to expansion context in whole file transformation
  callbacks of `register_transformation` (ocaml-ppx/ppxlib#202, @pitag-ha)

- Driver: take `-cookie` argument into account, also when the input is a
  binary AST (@pitag-ha, ocaml-ppx/ppxlib#209)

- `run_as_ppx_rewriter`: take into account the arguments
  `-loc-filename`, `apply` and `dont-apply` (ocaml-ppx/ppxlib#205, @pitag-ha)

- Location.Error: add functions `raise` and `update_loc`
  (ocaml-ppx/ppxlib#205, @pitag-ha)
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

Successfully merging this pull request may close these issues.

2 participants