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

[patch] Clarify cover string argument as comment not message. #64

Merged

Conversation

dtzSiFive
Copy link
Member

Tweak the existing field to match its use in practice (SFC), and planned to soon behave similarly in MFC: llvm/circt#4502 .

See #53 for discussion of this field, for now let's keep existing code working and update spec to reflect current behavior (and provide users a means to attach string information to the output that's not limited like optional_name).

@dtzSiFive dtzSiFive changed the title cover: message -> comment [patch] cover: message -> comment Jan 20, 2023
This is different than the explanatory message argument on
assert and assume statements.
@dtzSiFive dtzSiFive force-pushed the feature/cover-comment-not-message branch from e90ebc8 to 58b9779 Compare January 20, 2023 16:18
@dtzSiFive dtzSiFive changed the title [patch] cover: message -> comment [patch] Clarify cover string argument as comment not message. Jan 20, 2023
Copy link
Member

@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.

Good to define this existing behavior. 👍 LGTM

@dtzSiFive dtzSiFive merged commit 0190aba into chipsalliance:main Jan 23, 2023
@dtzSiFive dtzSiFive deleted the feature/cover-comment-not-message branch January 23, 2023 15:00
seldridge pushed a commit that referenced this pull request Mar 2, 2023
This is different than the explanatory message argument on
assert and assume statements.
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.

2 participants