Skip to content

Commit

Permalink
Add Wcase-dup warning
Browse files Browse the repository at this point in the history
  • Loading branch information
MikePopoloski committed Nov 23, 2024
1 parent 59d214e commit 2f86786
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 10 deletions.
9 changes: 5 additions & 4 deletions scripts/diagnostics.txt
Original file line number Diff line number Diff line change
Expand Up @@ -776,10 +776,7 @@ warning bitwise-op-mismatch BitwiseOpMismatch "bitwise operation between operand
warning comparison-mismatch ComparisonMismatch "comparison between operands of different types ({} and {})"
warning sign-compare SignCompare "comparison of differently signed types ({} and {})"
warning case-type CaseTypeMismatch "comparison of different types in '{}' statement ({} and {})"
warning case-default CaseDefault "'{}' missing 'default' label"
warning case-outside-range CaseOutsideRange "'{}' item with {} bits can never match the {} bit case expression"
warning case-enum CaseEnum "enumeration {} not handled in case statement"
warning case-enum-explicit CaseEnumExplicit "enumeration {} not explicitly handled in case statement"
warning bitwise-rel-precedence BitwiseRelPrecedence "'{}' has lower precedence than '{}'; '{}' will be evaluated first"
note NotePrecedenceBitwiseFirst "place parentheses around the '{}' expression to evaluate it first"
note NotePrecedenceSilence "place parentheses around the '{}' expression to silence this warning"
Expand Down Expand Up @@ -872,6 +869,10 @@ error BlockingInAlwaysFF "always_ff procedures cannot have blocking timing contr
error AlwaysFFEventControl "always_ff procedure must have one and only one event control"
error SeqEmptyMatch "sequence must not admit an empty match"
error SeqOnlyEmpty "sequence admits only empty matches"
warning case-default CaseDefault "'{}' missing 'default' label"
warning case-enum CaseEnum "enumeration {} not handled in case statement"
warning case-enum-explicit CaseEnumExplicit "enumeration {} not explicitly handled in case statement"
warning case-dup CaseDup "'{}' statement contains duplicate items for value '{}' (the second one will never be matched)"
warning seq-no-match SeqNoMatch "sequence can never be matched"
warning event-const EventExpressionConstant "edge expression is constant"
warning empty-stmt EmptyStatement "extra ';' has no effect"
Expand Down Expand Up @@ -1174,7 +1175,7 @@ group extra = { empty-member empty-stmt dup-import pointless-void-cast case-gen-
ineffective-sign port-width-trunc constant-conversion float-bool-conv int-bool-conv
useless-cast unsigned-arith-shift static-init-order static-init-value float-int-conv
float-narrow bitwise-rel-precedence arith-in-shift logical-not-parentheses
consecutive-comparison case-outside-range case-enum }
consecutive-comparison case-outside-range case-enum case-dup }

group pedantic = { empty-pattern implicit-net-port nonstandard-escape-code nonstandard-generate format-multibit-strength
nonstandard-sys-func nonstandard-foreach nonstandard-dist isunbounded-param-arg udp-coverage }
Expand Down
17 changes: 17 additions & 0 deletions scripts/warning_docs.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1561,3 +1561,20 @@ module m;
end
endmodule
```

-Wcase-dup
A case statement contains more than one item with the same value.
Case items are evaluated and selected in order and so the second
one will never be matched.
```
module m;
int i;
initial begin
case (i)
1, 2:;
3, 1:;
default;
endcase
end
endmodule
```
19 changes: 13 additions & 6 deletions source/ast/Statements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1201,14 +1201,21 @@ Statement& CaseStatement::fromSyntax(Compilation& compilation, const CaseStateme
<< LexerFacts::getTokenKindText(keyword);
}

if (expr->type->isEnum()) {
SmallSet<ConstantValue, 4> elems;
for (auto& g : result->items) {
for (auto item : g.expressions) {
if (auto cv = context.tryEval(*item))
elems.emplace(std::move(cv));
SmallMap<ConstantValue, const Expression*, 4> elems;
for (auto& g : result->items) {
for (auto item : g.expressions) {
if (auto cv = context.tryEval(*item)) {
auto [it, inserted] = elems.emplace(std::move(cv), item);
if (!inserted) {
auto& diag = context.addDiag(diag::CaseDup, item->sourceRange);
diag << LexerFacts::getTokenKindText(keyword) << it->first;
diag.addNote(diag::NotePreviousUsage, it->second->sourceRange);
}
}
}
}

if (expr->type->isEnum()) {

SmallVector<std::string_view> missing;
for (auto& ev : expr->type->getCanonicalType().as<EnumType>().values()) {
Expand Down
29 changes: 29 additions & 0 deletions tests/unittests/ast/WarningTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1181,3 +1181,32 @@ endmodule
CHECK(diags[10].code == diag::CaseDefault);
CHECK(diags[11].code == diag::CaseEnum);
}

TEST_CASE("Case statement dups") {
auto tree = SyntaxTree::fromText(R"(
module m;
int i;
event e;
initial begin
case (i)
1, 2:;
3, 1:;
default;
endcase
case (e)
null:;
null:;
default;
endcase
end
endmodule
)");

Compilation compilation;
compilation.addSyntaxTree(tree);

auto& diags = compilation.getAllDiagnostics();
REQUIRE(diags.size() == 2);
CHECK(diags[0].code == diag::CaseDup);
CHECK(diags[1].code == diag::CaseDup);
}

0 comments on commit 2f86786

Please sign in to comment.