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

Rule/explicit begin #2247

Merged
merged 17 commits into from
Sep 18, 2024
Merged

Conversation

sconwayaus
Copy link
Contributor

This PR forks the initial work started in PR #1336 and extends the rule to check for begin-end blocks for all if, else, always, always_comb, always_latch, always_ff, for, forever, foreach, while and initial statements.

The rule is configurable allowing users to enable/disable this rule for each statement to provide some level of flexibility.

Aside from the usual unit testing, I've also spent the last 3 months using this rule in anger and it seems to be working quite well.

Resolves #2040
Related to #1321, misses on the case statement suggestion.

sconwayaus and others added 16 commits April 6, 2024 21:36
Copied the code from suzizecat@0b087d7
and rebased it to the head. Updated the tests and formatted.
Add rule to check if the begin keyword always follows a if/else/for.
Related to chipsalliance#1321
Remove the fix suggestion as invalid
also move the error anchor to offending if/else/for in order to
allow test to pass
Merge branch 'rule/explicit-begin' of https://github.com/sconwayaus/verible into rule/explicit-begin
Merge branch 'rule/explicit-begin' of https://github.com/sconwayaus/verible into rule/explicit-begin
…aces instead of begin-end. To keep things simple, this change ignores all statements inside a constraint.
@sconwayaus sconwayaus force-pushed the rule/explicit-begin branch 11 times, most recently from aefa9e2 to 33d5ed0 Compare September 6, 2024 11:54
@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 95.93023% with 7 lines in your changes missing coverage. Please review.

Project coverage is 92.95%. Comparing base (c11e607) to head (6da2fe5).
Report is 28 commits behind head on master.

Files with missing lines Patch % Lines
verilog/analysis/checkers/explicit_begin_rule.cc 95.93% 7 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2247      +/-   ##
==========================================
+ Coverage   92.91%   92.95%   +0.04%     
==========================================
  Files         359      360       +1     
  Lines       26758    27039     +281     
==========================================
+ Hits        24863    25135     +272     
- Misses       1895     1904       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@hzeller hzeller left a comment

Choose a reason for hiding this comment

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

Very cool!
I need to look at it more in detail later this week.

@hzeller
Copy link
Collaborator

hzeller commented Sep 17, 2024

Adding @fangism as he always enjoys looking at new rules.

Copy link
Collaborator

@hzeller hzeller left a comment

Choose a reason for hiding this comment

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

Thanks, this is really cool!

@hzeller hzeller merged commit 88bf4fb into chipsalliance:master Sep 18, 2024
34 checks passed
@sconwayaus sconwayaus deleted the rule/explicit-begin branch September 18, 2024 11:05
@hzeller
Copy link
Collaborator

hzeller commented Sep 18, 2024

Looks like the automatic generation of the rule documentation picked this up https://chipsalliance.github.io/verible/lint.html#explicit-begin

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.

How to use rule for having a begin and end
3 participants