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

linter: do not require 'default' case for 'unique case' #2000

Merged

Conversation

IEncinas10
Copy link
Collaborator

Fixes #1278

Currently, case-missing-default rule requires the default case for every case statement. This PR makes an exception for unique case statements as requested by the issue.

I also changed where the diagnostics point to. Previously they pointed to the leftmost token of the kCaseItemList, but now they point to the case keyword.

case-missing-default rule required the 'default' case for every
case statement. This commit changes it so that case statements
with the 'unique' qualifier are not marked as errors by this
check.
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (0b8cb4e) 92.85% compared to head (ab245b5) 92.85%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2000   +/-   ##
=======================================
  Coverage   92.85%   92.85%           
=======================================
  Files         355      355           
  Lines       26272    26273    +1     
=======================================
+ Hits        24395    24396    +1     
  Misses       1877     1877           
Files Changed Coverage Δ
...log/analysis/checkers/case_missing_default_rule.cc 100.00% <100.00%> (ø)

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

@hzeller
Copy link
Collaborator

hzeller commented Aug 20, 2023

(currently on vacation, slow to respond with less internet bandwidth; will be back in a week or so for review)

@IEncinas10
Copy link
Collaborator Author

IEncinas10 commented Aug 21, 2023

Thanks for the heads up. Enjoy the vacation!

@goekce
Copy link

goekce commented Apr 15, 2024

@hzeller This patch is waiting for ~8 months. I would be glad to see this patch in verible.

@IEncinas10
Copy link
Collaborator Author

@hzeller This patch is waiting for ~8 months. I would be glad to see this patch in verible.

I'm sure he'll review it when he feels like it. People are busy, things take time.

Please be patient. If you need this feature, rebase this branch onto master and compile the project for yourself.

@hzeller
Copy link
Collaborator

hzeller commented Apr 15, 2024

Ah sorry everyone. This fell off my radar. Will see if I can make some time to review.

@IEncinas10
Copy link
Collaborator Author

Ah sorry everyone. This fell off my radar. Will see if I can make some time to review.

There is no hurry at all, thanks for all your work :)

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.

LGTM. Sorry for the deiay, sometimes things are busy ...

@hzeller hzeller merged commit f915e22 into chipsalliance:master May 27, 2024
34 of 35 checks passed
@IEncinas10 IEncinas10 deleted the fix-case-missing-default-rule branch May 28, 2024 18:53
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.

unique case should not require default
4 participants