pest icon indicating copy to clipboard operation
pest copied to clipboard

Debugger tweaks, added tracing

Open rlpowell opened this issue 11 months ago • 14 comments

  • Fixed debugger argument handling
  • Expanded debugger behaviour a bit (like being able to continue a bunch of times)
  • Added support for tracing the productions of a grammar during parsing, in both the debugger and normal modes

Commentary on this as a PR:

  • Just for your interest: I really needed to add tracing to debug why what is probably the largest PEG grammar in existence ( https://github.com/lojban/ilmentufa/blob/master/camxes.peg ) is parsing things very differently with Pest vs. the other parsers we've used
  • I've never done macros before and I've never worked with pest before; if I should do something differently, let me know
  • It was not at all obvious to me how to run all the tests in all the places in this repo, so I didn't; please guide me. Unless the PR process already does the tests, I guess.
  • Similarly, I'd love to set up a test that runs a simple grammar through both the debugger and the regular flow and shows that their tracing output is the same, but I don't really have any good ideas for how to do that besides a shell script.
  • This is kind of two things in one, but they're tied together so I'd rather not split them unless it's a big deal
  • My changes to the debugger had a mid-air collision with Jonas's, sorry about that; I believe my changes do all the things they added, though

Summary by CodeRabbit

  • New Features

    • Introduced specialized parser debugger and optional tracing output.
    • Enhanced command-line interface with new options for grammar, input, session files, and tracing configurations.
    • Added tracing capabilities to the parser, allowing for detailed debugging and analysis.
  • Documentation

    • Updated user guides to include a new tracing section with examples, detailing how to enable and leverage the improved tracing features.

rlpowell avatar Feb 05 '25 01:02 rlpowell

Walkthrough

This pull request introduces extensive tracing capabilities across the project. It updates documentation by adding new features and sections, refines the debugger to incorporate tracing options into its context and CLI handling via the clap library, and enhances code generation to support tracing configurations. Additionally, modifications in the parser and virtual machine components ensure that tracing is integrated into parsing routines, with new configurations, wrappers, and extended dependency specifications.

Changes

File(s) Change Summary
README.md, derive/src/lib.rs Updated documentation: Added “Specialized parser debugger”, “Optional tracing output”, new tracing section, header hierarchy adjustment, and updated derive attributes to include tracing.
debugger/Cargo.toml, debugger/src/lib.rs, debugger/src/main.rs Improved debugger: Added new dependency (clap), introduced tracing configuration field and methods in DebuggerContext, and refactored CLI handling with new structs and increased timeout.
generator/Cargo.toml, generator/src/generator.rs, generator/src/macros.rs, generator/src/parse_derive.rs Enhanced generator: Updated dependency for syn, added a tracing_config parameter to generation functions, modified macros (additional parameter for implicit), and updated parsing to extract and handle tracing attributes.
pest/Cargo.toml, pest/src/lib.rs, pest/src/parser_state.rs, pest/src/iterators/queueable_token.rs Upgraded parser: Added new dependencies (quote, proc-macro2), introduced TracingConfig and TracingType, updated exports and parser state creation with tracing wrappers, and fixed a minor documentation typo.
vm/src/lib.rs Modified virtual machine: Updated parsing flow to use parse_with_tracing, integrating tracing configurations into rule processing and error handling.

Suggested reviewers

  • NoahTheDuke
  • tomtau

Poem

I’m a little rabbit, hopping with cheer,
New tracing trails shine bright and clear,
Debuggers and parsers now dance in sync,
Code leaps forward in just a blink,
With CLI magic and a trace so fine,
Our project’s hopping on a brilliant line!
🐰✨

[!TIP]

🌐 Web search-backed reviews and chat
  • We have enabled web search-based reviews and chat for all users. This feature allows CodeRabbit to access the latest documentation and information on the web.
  • You can disable this feature by setting web_search: false in the knowledge_base settings.
  • Please share any feedback in the Discord discussion.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

coderabbitai[bot] avatar Feb 05 '25 01:02 coderabbitai[bot]

Ah, running cargo test at the top actually runs everything looks like; working on cleaning up the tests now.

rlpowell avatar Feb 05 '25 04:02 rlpowell

This PR contains many semver-breaking changes -- I suggest either changing it to be major version non-breaking or putting it under a feature flag

This should change no behaviour for normal users, except users of the debugger. What semver-breaking changes are you pointing at? (Also, I'm in the discord if you want to chat there.)

rlpowell avatar Feb 05 '25 04:02 rlpowell

OK, all the tests should pass now except that tests::json_handles_deep_nesting has a stack overflow; they all pass when I run RUST_MIN_STACK=16777216 cargo test though. Not sure what to do about that. Haven't gone over the coderabbit nits yet.

rlpowell avatar Feb 05 '25 04:02 rlpowell

Replying to coderabbit comments that I think might be important but I don't have good solutions for:

vm/src/lib.rs

326-326: Thirty-second timeout for requests is a reasonable fallback.

Increasing from 5s to 30s for network requests can significantly improve reliability for slower hosts. Still, consider user-configurable timeouts for maximum flexibility.

This was done solely because the grammar I'm using Very Large; people should object if it feels like a problem.

pest/src/parser_state.rs

24-26: Using std::println! might be too intrusive. Relying on println! for tracing output can clutter STDOUT. Consider a logging approach (e.g., a configurable logger) to avoid mixing parser messages with user-facing console output.

This is definitely something people should express opinions about if they have them.

generator/src/macros.rs 12-12: Consider verifying the boolean nature of the second macro parameter.

Passing false for $implicit here works fine as an example, but notice $implicit: ident doesn’t inherently enforce a boolean type. If you accidentally pass a non-boolean token, the macro won’t catch it. A more explicit naming or usage pattern might improve safety and clarity.

Yeah I have no idea what a better fix looks like here.

rlpowell avatar Feb 05 '25 05:02 rlpowell

I believe I've fixed everything obviously-fixable in coderabbit's comments, but see my long comment just above.

rlpowell avatar Feb 05 '25 05:02 rlpowell

This should change no behaviour for normal users, except users of the debugger. What semver-breaking changes are you pointing at? (Also, I'm in the discord if you want to chat there.)

@rlpowell I only had a quick look and I saw that pub fn new(... added parameters, but that's been fixed. Now, it looks better, but I haven't looked in detail -- especially I'm not sure about the derive-macro related changes. In general, it should be fine, but one issue I encountered before was that cargo can (for whatever mysterious dependency resolution reasons) decide to include different versions of pest crates (e.g. pest 2.7.15 and pest_generator 2.5.7), so some of these "internal cross-crate" changes could accidentally break the end user code.

tomtau avatar Feb 05 '25 07:02 tomtau

I see that some of the tests are failing because I'm using println!. I've never worked in a no-std environment before; is there a standard fix for that sort of thing?

rlpowell avatar Feb 09 '25 23:02 rlpowell

And I don't understand the errors at https://github.com/pest-parser/pest/actions/runs/13150923489/job/36701824224?pr=1080 at all.

rlpowell avatar Feb 09 '25 23:02 rlpowell

@rlpowell I guess those no_std errors are due to quote and proc-macro2 added to pest's Cargo.toml

tomtau avatar Feb 10 '25 11:02 tomtau

@tomtau To be clear I have no idea how to fix those errors; can you tell me how to run the same tests locally or something, or make some suggestions on how to fix them?

rlpowell avatar Mar 21 '25 04:03 rlpowell

@rlpowell Could quote and proc-macro2 be removed from pest/Cargo.toml ?

you can test it like this: cd pest && cargo build -j1 -Z build-std=core,alloc --no-default-features --target x86_64-unknown-linux-gnu with the nightly toolchain

tomtau avatar Mar 21 '25 06:03 tomtau

@rlpowell Could quote and proc-macro2 be removed from pest/Cargo.toml ?

I have no idea how. Two things:

  1. I've never dealt with macros before, and it took me hours to get these working properly; if there's a way to do it without quote and proc-macro2, I have no idea what that way would be, and I'd need some pointers.
  2. If there's something that I can use besides println to output the tracing itself, I don't know what that would be. I suppose I could somehow pass the tracing info back to something that isn't no_std? That problem seems like something I could maybe figure out. The macro stuff does not.

rlpowell avatar Mar 23 '25 06:03 rlpowell

@rlpowell

if there's a way to do it without quote and proc-macro2, I have no idea what that way would be, and I'd need some pointers.

Could for example that functionality that depends on those crates in pest be moved to pest_generator?

tomtau avatar Mar 23 '25 11:03 tomtau