From 59d214e1eb2ab0a7c3777b9976770fe35c0c35b0 Mon Sep 17 00:00:00 2001 From: MikePopoloski Date: Sat, 23 Nov 2024 11:03:42 -0500 Subject: [PATCH] Add Wcase-enum and Wcase-enum-explicit warnings --- scripts/diagnostics.txt | 4 +- scripts/warning_docs.txt | 31 +++++++++++++++ source/ast/Statements.cpp | 41 ++++++++++++++++++++ tests/unittests/ast/WarningTests.cpp | 57 ++++++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 1 deletion(-) diff --git a/scripts/diagnostics.txt b/scripts/diagnostics.txt index 747c1438b..0255ab854 100644 --- a/scripts/diagnostics.txt +++ b/scripts/diagnostics.txt @@ -778,6 +778,8 @@ 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" @@ -1172,7 +1174,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 } + consecutive-comparison case-outside-range case-enum } 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 } diff --git a/scripts/warning_docs.txt b/scripts/warning_docs.txt index 31c4a73da..2078bcace 100644 --- a/scripts/warning_docs.txt +++ b/scripts/warning_docs.txt @@ -1530,3 +1530,34 @@ module m; end endmodule ``` + +-Wcase-enum +A case statement with an enum type condition does not include items +that cover every enumerand. Note that this warning will not be +issued if there is a 'default' label. +``` +module m; + enum {A, B} e; + initial begin + case (e) + A: ; + endcase + end +endmodule +``` + +-Wcase-enum-explicit +A case statement with an enum type condition does not include items +that cover every enumerand, regardless of whether there is a default +label or not. +``` +module m; + enum {A, B} e; + initial begin + case (e) + A: ; + default; + endcase + end +endmodule +``` diff --git a/source/ast/Statements.cpp b/source/ast/Statements.cpp index 1f9d94a5b..6f26677ec 100644 --- a/source/ast/Statements.cpp +++ b/source/ast/Statements.cpp @@ -7,6 +7,8 @@ //------------------------------------------------------------------------------ #include "slang/ast/Statements.h" +#include + #include "slang/ast/ASTSerializer.h" #include "slang/ast/ASTVisitor.h" #include "slang/ast/Compilation.h" @@ -1199,6 +1201,45 @@ Statement& CaseStatement::fromSyntax(Compilation& compilation, const CaseStateme << LexerFacts::getTokenKindText(keyword); } + if (expr->type->isEnum()) { + SmallSet elems; + for (auto& g : result->items) { + for (auto item : g.expressions) { + if (auto cv = context.tryEval(*item)) + elems.emplace(std::move(cv)); + } + } + + SmallVector missing; + for (auto& ev : expr->type->getCanonicalType().as().values()) { + auto& val = ev.getValue(); + if (!elems.contains(val)) + missing.push_back(ev.name); + } + + if (!missing.empty()) { + std::string msg; + if (missing.size() == 1) { + msg = fmt::format("value '{}'", missing[0]); + } + else if (missing.size() == 2) { + msg = fmt::format("values '{}' and '{}'", missing[0], missing[1]); + } + else if (missing.size() == 3) { + msg = fmt::format("values '{}', '{}', and '{}'", missing[0], missing[1], + missing[2]); + } + else { + const size_t remainder = missing.size() - 3; + msg = fmt::format("values '{}', '{}', '{}' (and {} other{})", missing[0], + missing[1], missing[2], remainder, remainder == 1 ? "" : "s"); + } + + auto code = defStmt ? diag::CaseEnumExplicit : diag::CaseEnum; + context.addDiag(code, expr->sourceRange) << msg; + } + } + return *result; } diff --git a/tests/unittests/ast/WarningTests.cpp b/tests/unittests/ast/WarningTests.cpp index e8dce0410..b90f2a066 100644 --- a/tests/unittests/ast/WarningTests.cpp +++ b/tests/unittests/ast/WarningTests.cpp @@ -1124,3 +1124,60 @@ endmodule CHECK(diags[5].code == diag::CaseTypeMismatch); CHECK(diags[6].code == diag::CaseTypeMismatch); } + +TEST_CASE("Case statement missing enum values") { + auto tree = SyntaxTree::fromText(R"( +module m; + enum {A,B,C,D,E} e; + initial begin + case (e) + A, B, C, D:; + default; + endcase + case (e) + A, B, C:; + default; + endcase + case (e) + A, B:; + default; + endcase + case (e) + A:; + default; + endcase + + case (e) + A, B, C, D:; + endcase + case (e) + A, B, C:; + endcase + case (e) + A, B:; + endcase + case (e) + A:; + endcase + end +endmodule +)"); + + Compilation compilation; + compilation.addSyntaxTree(tree); + + auto& diags = compilation.getAllDiagnostics(); + REQUIRE(diags.size() == 12); + CHECK(diags[0].code == diag::CaseEnumExplicit); + CHECK(diags[1].code == diag::CaseEnumExplicit); + CHECK(diags[2].code == diag::CaseEnumExplicit); + CHECK(diags[3].code == diag::CaseEnumExplicit); + CHECK(diags[4].code == diag::CaseDefault); + CHECK(diags[5].code == diag::CaseEnum); + CHECK(diags[6].code == diag::CaseDefault); + CHECK(diags[7].code == diag::CaseEnum); + CHECK(diags[8].code == diag::CaseDefault); + CHECK(diags[9].code == diag::CaseEnum); + CHECK(diags[10].code == diag::CaseDefault); + CHECK(diags[11].code == diag::CaseEnum); +}