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

The "ocaml" toplevel should accept "-verbose" #7099

Closed
vicuna opened this issue Dec 17, 2015 · 11 comments · May be fixed by #10814
Closed

The "ocaml" toplevel should accept "-verbose" #7099

vicuna opened this issue Dec 17, 2015 · 11 comments · May be fixed by #10814

Comments

@vicuna
Copy link

vicuna commented Dec 17, 2015

Original bug ID: 7099
Reporter: @alainfrisch
Status: acknowledged (set by @damiendoligez on 2016-11-08T10:30:45Z)
Resolution: open
Priority: low
Severity: feature
Category: toplevel
Tags: junior_job
Monitored by: @nojb @gasche @diml

Bug description

This would be useful to trace calls to external ppx programs. In "-verbose" mode, the toplevel should perhaps print the "context" passed to and received from ppxs, in particular the "cookies" field. This could be very useful to track bugs such as #7098 or other bugs in ppxs themselves.

[Edit: I initially wrote "-v" but of course I meant "-verbose" as for ocamlc/ocamlopt.]

@vicuna
Copy link
Author

vicuna commented Dec 17, 2015

Comment author: @gasche

When I saw the mantis ticket title, I thought that this would be a synonym to -version. Maybe -debug or -verbose would be better names.

I do some amount of debugging in the toplevel using -dsource or -dparsetree. Would it be possible for a refinement of those options (for example, having options to print the source before and after preprocessing, and similarly for parsetree) support your use-case? They could also be useful for debugging interactions with ocamlc.

@vicuna
Copy link
Author

vicuna commented Dec 17, 2015

Comment author: @alainfrisch

Indeed, being able to show the AST sent to or received from each PPX would be useful (that's how I debugged #7098). Should this be a new option, or just triggered when both -dparse (resp. -dsource) and -verbose are enabled?

@vicuna
Copy link
Author

vicuna commented Mar 1, 2019

Comment author: kaaira

can i please work on this? do reply so that i can know if i need to look for some other issue. Thanks.

@vicuna
Copy link
Author

vicuna commented Mar 1, 2019

Comment author: @alainfrisch

Yes, feel free to pick up this work if you interested in it!

@vicuna
Copy link
Author

vicuna commented Mar 1, 2019

Comment author: @gasche

In case this can help, here is maybe a more detailed description of what the issue is about.

Currently if you run "ocaml -dsource" and enter the following commands:

#use "topfind";; (* this enables lookup of findlib packages *)
#require "ppx_deriving.show";; (* this enables the ppx_deriving ppx *)
type t = Foo of int [@@deriving show];;

then the last line will show the source code interpreted by the toplevel, that is, the source generated by the ppx_deriving preprocessor. This is useful for users wishing to understand what the ppx does, but not so much for people trying to debug it when it behaves wrongly, which would also need to see the input sent to the ppx by the toplevel.

Alain is proposing that such a mode be enabled when both -dsource and -verbose are active. (Another option different from -dsource is -dparsetree, which shows the complete AST rather than the concrete syntax, and it would behave similarly.) So the output of the last line could be something like

(* content sent to the ppx processor "ppx_deriving" *)
<source code before processing, including ppx-context>

(* content receive from the ppx processor "ppx_deriving" *)
<source code after processing, including ppx-context>

Final tips:

  • The code that applies the ppx rewriters is in driver/pparse.
  • What Alain calls the "cookies" above is special metadata exchanged by the compiler and the ppx processor, as part of the "ppx context". The compiler tells the preprocessor about compilation options, and the preprocessor also return similar metadata.
  • While driver/pparse contains the high-level logic for preprocessors, most of the logic is actually in parsing/ast_mapper, in particular the PpxContext submodule which contains the cookie logic.

My gut feeling, but I'm not sure, is that it should suffice to modify driver/pparse to implement this logic (in particular the "apply_rewriters_*" functions), without touching parsing/ast_mapper, but reading that file may still be useful to understand the context.

@ulugbekna
Copy link
Contributor

I'd want to work on this if no one is already.

@nojb
Copy link
Contributor

nojb commented Mar 25, 2019

Go for it!

Just for information, someone else had expressed interest (user kaaira above), but I don't think there has been any news since.

@github-actions
Copy link

github-actions bot commented May 9, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 9, 2020
@gasche gasche removed the Stale label May 9, 2020
@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 12, 2021
@gasche gasche removed the Stale label Jun 16, 2021
@gasche gasche reopened this Jun 16, 2021
@gasche
Copy link
Member

gasche commented Jun 16, 2021

This remains a useful feature request and a valuable "newcomer job".

@github-actions
Copy link

github-actions bot commented Jul 1, 2022

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label Jul 1, 2022
@github-actions github-actions bot closed this as completed Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants