Skip to content

Commit

Permalink
Add Wcase-not-wildcard and Wcasez-with-x warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
MikePopoloski committed Nov 25, 2024
1 parent 5399d34 commit 2bc2126
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 3 deletions.
2 changes: 2 additions & 0 deletions scripts/diagnostics.txt
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,8 @@ 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 case-overlap CaseOverlap "'{}' statement contains overlapping items"
warning case-not-wildcard CaseNotWildcard "'case' statement item has unknown bits which don't work as wildcards; should this be a 'casex' or 'casez' statement?"
warning casez-with-x CaseZWithX "'casez' statement item has 'x' bits which don't work as wildcards; should this be a 'casex' statement?"
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
30 changes: 30 additions & 0 deletions scripts/warning_docs.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1594,3 +1594,33 @@ module m;
end
endmodule
```

-Wcase-not-wildcard
A normal case statement has items with integer literals that contain X or Z bits.
These bits don't count as wildcards, so the designer may have meant for this to
be a 'casex' or 'casez' statement instead.
```
module m;
logic [2:0] a;
initial begin
case (a)
3'b10?:;
endcase
end
endmodule
```

-Wcasez-with-x
A 'casez' statement has items with integer literals that contain X bits.
These bits don't count as wildcards, so the designer may have meant for this to
be a 'casex' statement instead.
```
module m;
logic [2:0] a;
initial begin
casez (a)
3'b10x:;
endcase
end
endmodule
```
20 changes: 20 additions & 0 deletions source/ast/statements/ConditionalStatements.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,26 @@ Statement& CaseStatement::fromSyntax(Compilation& compilation, const CaseStateme
}
}
}

if (condition == CaseStatementCondition::Normal ||
condition == CaseStatementCondition::WildcardJustZ) {
// If we're not in a wildcard case we should warn
// about integer literal items that have unknown bits.
// Similarly, if we're in a wildcard case with just Zs
// we should warn if we see Xs.
auto& unwrapped = item->unwrapImplicitConversions();
if (unwrapped.kind == ExpressionKind::IntegerLiteral) {
auto& lit = unwrapped.as<IntegerLiteral>();
if (condition == CaseStatementCondition::Normal &&
lit.getValue().hasUnknown()) {
context.addDiag(diag::CaseNotWildcard, item->sourceRange);
}
else if (condition == CaseStatementCondition::WildcardJustZ &&
lit.getValue().countXs() > 0) {
context.addDiag(diag::CaseZWithX, item->sourceRange);
}
}
}
}
}

Expand Down
36 changes: 33 additions & 3 deletions tests/unittests/ast/WarningTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1241,10 +1241,40 @@ endmodule
compilation.addSyntaxTree(tree);

auto& diags = compilation.getAllDiagnostics();
REQUIRE(diags.size() == 5);
REQUIRE(diags.size() == 7);
CHECK(diags[0].code == diag::CaseOverlap);
CHECK(diags[1].code == diag::CaseOverlap);
CHECK(diags[2].code == diag::CaseOverlap);
CHECK(diags[3].code == diag::CaseOverlap);
CHECK(diags[4].code == diag::CaseOverlap);
CHECK(diags[3].code == diag::CaseZWithX);
CHECK(diags[4].code == diag::CaseZWithX);
CHECK(diags[5].code == diag::CaseOverlap);
CHECK(diags[6].code == diag::CaseOverlap);
}

TEST_CASE("Case items with unknowns that are not wildcards") {
auto tree = SyntaxTree::fromText(R"(
module m;
logic [3:0] a;
initial begin
case (a)
4'b?10:;
4'bx10:;
default;
endcase
casez (a)
4'bx10:;
default;
endcase
end
endmodule
)");

Compilation compilation;
compilation.addSyntaxTree(tree);

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

0 comments on commit 2bc2126

Please sign in to comment.