Skip to content

Commit f915e22

Browse files
hzellerhzeller
authored
Merge pull request #2000 from IEncinas10/fix-case-missing-default-rule
linter: do not require 'default' case for 'unique case'
2 parents a416dee + ab245b5 commit f915e22

File tree

4 files changed

+149
-76
lines changed

4 files changed

+149
-76
lines changed

‎verilog/CST/verilog_matchers.h

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017-2020 The Verible Authors.
1+
// Copyright 2017-2023 The Verible Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -411,8 +411,32 @@ inline constexpr auto DeclarationDimensionsHasRanges =
411411
// endcase
412412
//
413413
inline constexpr auto HasDefaultCase =
414-
verible::matcher::MakePathMatcher(N(kDefaultItem));
414+
verible::matcher::MakePathMatcher(N(kCaseItemList), N(kDefaultItem));
415415

416+
// Matches with statements qualified with "unique"
417+
// For instance, matches:
418+
//
419+
// unique case (in)
420+
// default: return 0;
421+
// endcase
422+
//
423+
// unique if (a)
424+
// ...
425+
// else if (!a)
426+
// ...
427+
//
428+
// but not:
429+
//
430+
// case (in)
431+
// default: return 0;
432+
// endcase
433+
//
434+
// if (a)
435+
// ...
436+
// else if (!a)
437+
// ...
438+
inline constexpr auto HasUniqueQualifier =
439+
verible::matcher::MakePathMatcher(L(TK_unique));
416440
// Clean up macros
417441
#undef N
418442
#undef L

‎verilog/CST/verilog_matchers_test.cc

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017-2020 The Verible Authors.
1+
// Copyright 2017-2023 The Verible Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -788,5 +788,44 @@ TEST(VerilogMatchers, HasDefaultCaseTests) {
788788
}
789789
}
790790

791+
// Tests for HasUniqueQualifier matching.
792+
TEST(VerilogMatchers, HasUniqueQualifierTests) {
793+
const RawMatcherTestCase tests[] = {
794+
{HasUniqueQualifier(), "", 0},
795+
{HasUniqueQualifier(),
796+
R"(
797+
function automatic int foo (input in);
798+
case (in)
799+
default: return 0;
800+
endcase
801+
endfunction
802+
)",
803+
0},
804+
{HasUniqueQualifier(),
805+
R"(
806+
function automatic int foo (input in);
807+
unique case (in)
808+
1: return 0;
809+
endcase
810+
endfunction
811+
)",
812+
1},
813+
{HasUniqueQualifier(),
814+
R"(
815+
function automatic int foo (input in);
816+
unique if (in) begin
817+
return 0;
818+
end
819+
else if(!in) begin
820+
return 1;
821+
end
822+
endfunction
823+
)",
824+
1},
825+
};
826+
for (const auto &test : tests) {
827+
verible::matcher::RunRawMatcherTestCase<VerilogAnalyzer>(test);
828+
}
829+
}
791830
} // namespace
792831
} // namespace verilog

‎verilog/analysis/checkers/case_missing_default_rule.cc

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017-2020 The Verible Authors.
1+
// Copyright 2017-2023 The Verible Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -40,28 +40,39 @@ using verible::matcher::Matcher;
4040
VERILOG_REGISTER_LINT_RULE(CaseMissingDefaultRule);
4141

4242
static constexpr absl::string_view kMessage =
43-
"Explicitly define a default case for every case statement.";
43+
"Explicitly define a default case for every case statement or add `unique` "
44+
"qualifier to the case statement.";
4445

4546
const LintRuleDescriptor &CaseMissingDefaultRule::GetDescriptor() {
4647
static const LintRuleDescriptor d{
4748
.name = "case-missing-default",
4849
.topic = "case-statements",
49-
.desc = "Checks that a default case-item is always defined.",
50+
.desc =
51+
"Checks that a default case-item is always defined unless the case "
52+
"statement has the `unique` qualifier.",
5053
};
5154
return d;
5255
}
5356

54-
static const Matcher &CaseMatcher() {
55-
static const Matcher matcher(
56-
NodekCaseItemList(verible::matcher::Unless(HasDefaultCase())));
57-
return matcher;
58-
}
59-
6057
void CaseMissingDefaultRule::HandleSymbol(
6158
const verible::Symbol &symbol, const verible::SyntaxTreeContext &context) {
6259
verible::matcher::BoundSymbolManager manager;
63-
if (context.DirectParentIs(NodeEnum::kCaseStatement) &&
64-
CaseMatcher().Matches(symbol, &manager)) {
60+
61+
// Ensure that symbol is a kCaseStatement node
62+
if (symbol.Kind() != verible::SymbolKind::kNode) return;
63+
const verible::SyntaxTreeNode &node = verible::SymbolCastToNode(symbol);
64+
if (!node.MatchesTag(NodeEnum::kCaseStatement)) return;
65+
66+
static const Matcher uniqueCaseMatcher(
67+
NodekCaseStatement(HasUniqueQualifier()));
68+
69+
static const Matcher caseMatcherWithDefaultCase(
70+
NodekCaseStatement(HasDefaultCase()));
71+
72+
// If the case statement doesn't have the "unique" qualifier and
73+
// it is missing the "default" case, insert the violation
74+
if (!uniqueCaseMatcher.Matches(symbol, &manager) &&
75+
!caseMatcherWithDefaultCase.Matches(symbol, &manager)) {
6576
violations_.insert(LintViolation(symbol, kMessage, context));
6677
}
6778
}

‎verilog/analysis/checkers/case_missing_default_rule_test.cc

Lines changed: 61 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017-2020 The Verible Authors.
1+
// Copyright 2017-2023 The Verible Authors.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -42,7 +42,6 @@ TEST(CaseMissingDefaultRuleTest, BasicTests) {
4242
// Tests that the expected number of case-missing-default-rule violations inside
4343
// functions are found.
4444
TEST(CaseMissingDefaultRuleTest, CaseInsideFunctionTests) {
45-
// Currently, diagnostics point to the leftmost token of the kCaseItemList.
4645
const std::initializer_list<LintTestCase> kTestCases = {
4746
// case tests
4847
{R"(
@@ -54,10 +53,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideFunctionTests) {
5453
)"},
5554
{R"(
5655
function automatic int foo (input in);
57-
case (in)
58-
)",
59-
{TK_DecNumber, "1"},
60-
R"(: return 0;
56+
)",
57+
{TK_case, "case"},
58+
R"( (in)
59+
1: return 0;
6160
endcase
6261
endfunction
6362
)"},
@@ -72,10 +71,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideFunctionTests) {
7271
)"},
7372
{R"(
7473
function automatic int foo (input in);
75-
casex (in)
76-
)",
77-
{TK_DecNumber, "1"},
78-
R"(: return 0;
74+
)",
75+
{TK_casex, "casex"},
76+
R"( (in)
77+
1: return 0;
7978
endcase
8079
endfunction
8180
)"},
@@ -90,10 +89,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideFunctionTests) {
9089
)"},
9190
{R"(
9291
function automatic int foo (input in);
93-
casez (in)
94-
)",
95-
{TK_DecNumber, "1"},
96-
R"(: return 0;
92+
)",
93+
{TK_casez, "casez"},
94+
R"( (in)
95+
1: return 0;
9796
endcase
9897
endfunction
9998
)"},
@@ -127,10 +126,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideTaskTests) {
127126
)"},
128127
{R"(
129128
task automatic foo (input in);
130-
case (in)
131-
)",
132-
{TK_DecNumber, "1"},
133-
R"(: return 0;
129+
)",
130+
{TK_case, "case"},
131+
R"( (in)
132+
1: return 0;
134133
endcase
135134
endtask
136135
)"},
@@ -145,10 +144,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideTaskTests) {
145144
)"},
146145
{R"(
147146
task automatic foo (input in);
148-
casex (in)
149-
)",
150-
{TK_DecNumber, "1"},
151-
R"(: return 0;
147+
)",
148+
{TK_casex, "casex"},
149+
R"( (in)
150+
1: return 0;
152151
endcase
153152
endtask
154153
)"},
@@ -163,10 +162,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideTaskTests) {
163162
)"},
164163
{R"(
165164
task automatic foo (input in);
166-
casez (in)
167-
)",
168-
{TK_DecNumber, "1"},
169-
R"(: return 0;
165+
)",
166+
{TK_casez, "casez"},
167+
R"( (in)
168+
1: return 0;
170169
endcase
171170
endtask
172171
)"},
@@ -192,13 +191,13 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideFunctionTests) {
192191
const std::initializer_list<LintTestCase> kTestCases = {
193192
// Inner case doesn't have default, outer case does.
194193
{R"(
195-
function automatic int foo (input in);
194+
function automatic foo (input in);
196195
case (in)
197196
1: begin;
198-
case (in)
199-
)",
200-
{TK_DecNumber, "1"},
201-
R"(: return 1;
197+
)",
198+
{TK_case, "case"},
199+
R"( (in)
200+
1: return 1;
202201
endcase
203202
end
204203
default: return 1;
@@ -208,11 +207,11 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideFunctionTests) {
208207

209208
// Inner case does have default, outer case doesn't.
210209
{R"(
211-
function automatic int foo (input in);
212-
case (in)
213-
)",
214-
{TK_DecNumber, "1"},
215-
R"(: begin;
210+
function automatic foo (input in);
211+
)",
212+
{TK_case, "case"},
213+
R"( (in)
214+
1: begin;
216215
case (in)
217216
1: return 1;
218217
default: return 1;
@@ -224,7 +223,7 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideFunctionTests) {
224223

225224
// Both inner and outer case have default case statements.
226225
{R"(
227-
function automatic int foo (input in);
226+
function automatic foo (input in);
228227
case (in)
229228
1: begin;
230229
case (in)
@@ -239,15 +238,15 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideFunctionTests) {
239238

240239
// Neither of the cases have default case statements.
241240
{R"(
242-
function automatic int foo (input in);
243-
case (in)
244-
)",
245-
{TK_DecNumber, "1"},
246-
R"(: begin;
247-
case (in)
248-
)",
249-
{TK_DecNumber, "1"},
250-
R"(: return 1;
241+
function automatic foo (input in);
242+
)",
243+
{TK_case, "case"},
244+
R"( (in)
245+
1: begin;
246+
)",
247+
{TK_case, "case"},
248+
R"( (in)
249+
1: return 1;
251250
endcase
252251
end
253252
endcase
@@ -267,10 +266,10 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideTaskTests) {
267266
task automatic foo (input in);
268267
case (in)
269268
1: begin;
270-
case (in)
271-
)",
272-
{TK_DecNumber, "1"},
273-
R"(: return 1;
269+
)",
270+
{TK_case, "case"},
271+
R"( (in)
272+
1: return 1;
274273
endcase
275274
end
276275
default: return 1;
@@ -281,10 +280,10 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideTaskTests) {
281280
// Inner case does have default, outer case doesn't.
282281
{R"(
283282
task automatic foo (input in);
284-
case (in)
285-
)",
286-
{TK_DecNumber, "1"},
287-
R"(: begin;
283+
)",
284+
{TK_case, "case"},
285+
R"( (in)
286+
1: begin;
288287
case (in)
289288
1: return 1;
290289
default: return 1;
@@ -312,14 +311,14 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideTaskTests) {
312311
// Neither of the cases have default case statements.
313312
{R"(
314313
task automatic foo (input in);
315-
case (in)
316-
)",
317-
{TK_DecNumber, "1"},
318-
R"(: begin;
319-
case (in)
320-
)",
321-
{TK_DecNumber, "1"},
322-
R"(: return 1;
314+
)",
315+
{TK_case, "case"},
316+
R"( (in)
317+
1: begin;
318+
)",
319+
{TK_case, "case"},
320+
R"( (in)
321+
1: return 1;
323322
endcase
324323
end
325324
endcase

0 commit comments

Comments
 (0)