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

[P4fmt]: Add comment printing capability to P4Formatter #4887

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

snapdgn
Copy link
Contributor

@snapdgn snapdgn commented Aug 27, 2024

This builds on top off #4845(which traverses the AST and builds a side map for storing comments associated with each node). This PR uses that map to retrieve and print comments through the P4Formatter pretty-printer.

Support for new nodes are being added continuously.

Signed-off-by: Nitish <snapdgnn@proton.me>
Signed-off-by: Nitish <snapdgnn@proton.me>
Signed-off-by: Nitish <snapdgnn@proton.me>
Signed-off-by: Nitish <snapdgnn@proton.me>
Signed-off-by: Nitish <snapdgnn@proton.me>
@qobilidop
Copy link
Member

Is this expected to replace #4845?

@snapdgn
Copy link
Contributor Author

snapdgn commented Aug 27, 2024

Is this expected to replace #4845?

No. This builds on top of #4845, 4845 stores comments associated with each IR nodes, this PR implements the printing capabilities to the P4Formatter (p-printer) by using that side map.

I will rebase this as soon as 4845 is merged.

@snapdgn snapdgn changed the title Update P4Formatter to Include Attached Comments in Output Add comment printing capability to P4Formatter Aug 27, 2024
@fruffy fruffy changed the title Add comment printing capability to P4Formatter [P4fmt]: Add comment printing capability to P4Formatter Aug 28, 2024
Signed-off-by: Nitish <snapdgnn@proton.me>
@qobilidop qobilidop added the p4fmt Topics related to P4 formatter. label Sep 30, 2024
@fruffy
Copy link
Collaborator

fruffy commented Nov 7, 2024

@snapdgn Could you rebase this now that #4845 is merged?

@snapdgn
Copy link
Contributor Author

snapdgn commented Nov 7, 2024

@snapdgn Could you rebase this now that #4845 is merged?

Sorry. Was caught up in something.This needs a little work.
Will do this asap.
Update: Might need a little time on this one, Currently caught up in Semester Exams.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4fmt Topics related to P4 formatter.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants