-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add support for emitting string-valued plusargs and plusargs with no default #2453
Conversation
lgtm but @seldridge is the contributor and primary user of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good.
I have some inline comments on how to do this without breaking backwards compatibility.
Forgot to mention: Scaladoc for anything added in the PR would be good, too! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice implicit type pattern
Type of change: other enhancement
Impact: API modification
Development Phase: implementation
I came across a case where I needed to add a plusarg in a BlackBoxed verilog module and found that I needed to use
PlusArgArtefacts.append
for verilator to correctly parse my plusarg. This worked fine, but to get the help text to make sense I needed to add some features. This does change the API forPlusArgInfo
, but other function calls are backwards compatible.