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

[nfc] Add ability to test code snippets #172

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

seldridge
Copy link
Member

@seldridge seldridge commented Feb 12, 2024

This demonstrates a flow that can be used to validate that all code
snippets used in the FIRRTL specification correctly parse. This is
implemented using a simple pandoc filter to extract the bodies of all code
blocks with known languages (FIRRTL and Verilog), write them to the build
directory, and then run them all through an appropriate tool based on file
extension (firtool or verilator).

This may be a little cumbersome as every code example has to be completely
legal. To alleviate this, this PR includes the ability to add special
delimiters, indicated by lines with the text snippetbegin/snippetend which
will cause the filter to extract only the lines between the delimiters.

CC: @dtzSiFive

@seldridge seldridge force-pushed the dev/seldridge/test-code-examples branch 7 times, most recently from 28065f7 to a016c82 Compare February 12, 2024 04:07
@seldridge seldridge force-pushed the dev/seldridge/test-code-examples branch from a016c82 to 9e363b2 Compare February 12, 2024 04:09
@seldridge seldridge marked this pull request as ready for review February 12, 2024 04:14
@seldridge seldridge force-pushed the dev/seldridge/test-code-examples branch 4 times, most recently from e1cccfa to 60c018c Compare February 13, 2024 06:16
@seldridge
Copy link
Member Author

seldridge commented Feb 13, 2024

This now includes basic delimited support. The magic delimiters snippetbegin/snippetend can be used to extract out any number of code regions. E.g., this allows you to do things like:

FIRRTL version 4.0.0
circuit Foo:
  module Foo:
    input a: UInt<1>
    output b: UInt<1>

    ;; snippetbegin
    connect b, a
    ;; snippetend

This will extract the entire code block to a file. It will replace the original code block with:

    connect b, a

@seldridge seldridge force-pushed the dev/seldridge/test-code-examples branch from b9461ff to 2191922 Compare February 14, 2024 07:04
@mmaloney-sf
Copy link
Collaborator

Just to be sure, the line comment syntax is ; ... in FIRRTL and not // ... right? (Your example in the PR looks like it will error out. I don't know if that's material to the logic, though).

Copy link
Collaborator

@mwachs5 mwachs5 left a comment

Choose a reason for hiding this comment

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

could we add a few sentences to CONTRIBUTING.md that explains this a little bit?

@seldridge
Copy link
Member Author

Just to be sure, the line comment syntax is ; ... in FIRRTL and not // ... right? (Your example in the PR looks like it will error out. I don't know if that's material to the logic, though).

Yes! That's a mistake. The filter will still handle that, but that is illegal FIRRTL. The intent is that the code should put the snippetbegin/snippetend delimiters inside a comment.

@seldridge seldridge force-pushed the dev/seldridge/test-code-examples branch from 4ae925e to c869e65 Compare February 18, 2024 05:03
Copy link
Member Author

@seldridge seldridge left a comment

Choose a reason for hiding this comment

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

This is good for a final review. I will open a second PR which fixes all the code examples. We can then turn this on in CI.


local f = io.open(filename, 'w')
f:write(elem.text)
f:write("\n")
Copy link
Member Author

Choose a reason for hiding this comment

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

This newline helps for readability and avoids a Verilator lint warning that we don't have to waive.

systemverilog = "sv"
}
local ext = extMap[elem.classes[1]]
local skip = extMap[elem.classes[2]] == notest
Copy link
Member Author

Choose a reason for hiding this comment

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

This allows opting out from testing for a specific snippet using syntax like {.firrtl .notest} on the code block. This is very helpful for things that are really annoying to test like Verilog that includes input files or FIRRTL code blocks that are intentionally not legal through the use of pseudo-type parameters (see memories).

@seldridge seldridge force-pushed the dev/seldridge/test-code-examples branch 2 times, most recently from 9dc8b20 to cf436d0 Compare February 18, 2024 05:32
@seldridge
Copy link
Member Author

@mwachs5 wrote:

could we add a few sentences to CONTRIBUTING.md that explains this a little bit?

Added in the force push!

@seldridge seldridge force-pushed the dev/seldridge/test-code-examples branch 2 times, most recently from 9568f78 to 4629954 Compare February 18, 2024 22:45
Copy link
Collaborator

@mikeurbach mikeurbach left a comment

Choose a reason for hiding this comment

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

This is my first exposure to pandoc filters, so I don't have any strong comments on the lua implementation. The usage and goal makes sense to me in general.

This demonstrates a flow that can be used to validate that all code
snippets used in the FIRRTL specification correctly parse.  This is
implemented using a simple pandoc filter to extract the bodies of all code
blocks with known languages (FIRRTL and Verilog), write them to the build
directory, and then run them all through an appropriate tool based on file
extension (firtool or verilator).

This may be a little cumbersome as every code example has to be completely
legal.  To alleviate this, this PR includes the ability to add special
delimiters, indicated by lines with the text `snippetbegin`/`snippetend` which
will cause the filter to extract only the lines between the delimiters.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
@seldridge seldridge force-pushed the dev/seldridge/test-code-examples branch from 4629954 to fcbd3df Compare February 20, 2024 18:50
@seldridge seldridge merged commit fcbd3df into main Feb 20, 2024
1 check passed
@seldridge seldridge deleted the dev/seldridge/test-code-examples branch February 20, 2024 19:22
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.

4 participants