Add Wcase-overlap warning

scripts/diagnostics.txt

@@ -873,6 +873,7 @@ 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 case-overlap CaseOverlap "'{}' statement contains overlapping items"
 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"
@@ -1175,7 +1176,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 case-dup }
+                consecutive-comparison case-outside-range case-enum case-dup case-overlap }
 
 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 }

scripts/warning_docs.txt

@@ -1578,3 +1578,19 @@ module m;
     end
 endmodule
 ```
+
+-Wcase-overlap
+A wildcard case statement contains overlapping items, which can be confusing
+since the overlapping value(s) will only ever match the first item in the list.
+```
+module m;
+    logic [3:0] a;
+    initial begin
+        casez (a)
+            4'b1011:;
+            4'b101?:;
+            default;
+        endcase
+    end
+endmodule
+```

source/ast/statements/ConditionalStatements.cpp

@@ -192,6 +192,68 @@ void ConditionalStatement::serializeTo(ASTSerializer& serializer) const {
         serializer.write("ifFalse", *ifFalse);
 }
 
+// A trie for checking overlap of case items.
+class CaseTrie {
+public:
+    // Tries to insert the given value. If successful, nullptr is returned.
+    // Otherwise, the overlapping expression is returned.
+    const Expression* insert(const SVInt& value, const Expression& expr, bool wildcardX,
+                             BumpAllocator& alloc) {
+        if (auto result = find(value, 0, expr, wildcardX))
+            return result;
+
+        CaseTrie* curr = this;
+        const auto width = value.getBitWidth();
+        for (uint32_t i = 0; i < width; i++) {
+            const auto bit = value[i];
+            const bool isWildcard = wildcardX ? bit.isUnknown() : bit.value == logic_t::Z_VALUE;
+            const auto valueIndex = isWildcard ? 3 : bit.isUnknown() ? 2 : bit.value;
+
+            auto& elem = curr->nodes[valueIndex];
+            if (i == width - 1) {
+                elem.expr = &expr;
+            }
+            else {
+                if (!elem.trie)
+                    elem.trie = alloc.emplace<CaseTrie>();
+                curr = elem.trie;
+            }
+        }
+
+        return nullptr;
+    }
+
+private:
+    const Expression* find(const SVInt& value, uint32_t bitIndex, const Expression& expr,
+                           bool wildcardX) {
+        const auto bit = value[bitIndex];
+        const bool lastBit = bitIndex == value.getBitWidth() - 1;
+        const bool isWildcard = wildcardX ? bit.isUnknown() : bit.value == logic_t::Z_VALUE;
+        const auto valueIndex = bit.isUnknown() ? 2 : bit.value;
+
+        for (int i = 0; i < 4; i++) {
+            if (isWildcard || i == valueIndex || i == 3) {
+                if (lastBit) {
+                    if (nodes[i].expr)
+                        return nodes[i].expr;
+                }
+                else if (nodes[i].trie) {
+                    if (auto result = nodes[i].trie->find(value, bitIndex + 1, expr, wildcardX))
+                        return result;
+                }
+            }
+        }
+
+        return nullptr;
+    }
+
+    union Node {
+        CaseTrie* trie;
+        const Expression* expr;
+    };
+    Node nodes[4] = {};
+};
+
 static CaseStatementCondition getCondition(TokenKind kind) {
     CaseStatementCondition condition;
     switch (kind) {
@@ -308,6 +370,11 @@ Statement& CaseStatement::fromSyntax(Compilation& compilation, const CaseStateme
             << LexerFacts::getTokenKindText(keyword);
     }
 
+    CaseTrie trie;
+    std::optional<BumpAllocator> trieAlloc;
+    if (wildcard)
+        trieAlloc.emplace();
+
     SmallMap<ConstantValue, const Expression*, 4> elems;
     for (auto& g : result->items) {
         for (auto item : g.expressions) {
@@ -318,12 +385,20 @@ Statement& CaseStatement::fromSyntax(Compilation& compilation, const CaseStateme
                     diag << LexerFacts::getTokenKindText(keyword) << it->first;
                     diag.addNote(diag::NotePreviousUsage, it->second->sourceRange);
                 }
+                else if (wildcard) {
+                    if (auto prev = trie.insert(it->first.integer(), *item,
+                                                condition == CaseStatementCondition::WildcardXOrZ,
+                                                *trieAlloc)) {
+                        auto& diag = context.addDiag(diag::CaseOverlap, item->sourceRange)
+                                     << LexerFacts::getTokenKindText(keyword);
+                        diag.addNote(diag::NotePreviousUsage, prev->sourceRange);
+                    }
+                }
             }
         }
     }
 
     if (expr->type->isEnum()) {
-
         SmallVector<std::string_view> missing;
         for (auto& ev : expr->type->getCanonicalType().as<EnumType>().values()) {
             auto& val = ev.getValue();

tests/unittests/ast/StatementTests.cpp

@@ -318,7 +318,7 @@ module m;
         endcase
 
         casex (foo)
-            1'bx: ;
+            1'b0: ;
             1'd1: ;
             default;
         endcase

tests/unittests/ast/WarningTests.cpp

@@ -1115,14 +1115,15 @@ endmodule
     compilation.addSyntaxTree(tree);
 
     auto& diags = compilation.getAllDiagnostics();
-    REQUIRE(diags.size() == 7);
+    REQUIRE(diags.size() == 8);
     CHECK(diags[0].code == diag::CaseOutsideRange);
     CHECK(diags[1].code == diag::CaseTypeMismatch);
     CHECK(diags[2].code == diag::CaseTypeMismatch);
     CHECK(diags[3].code == diag::CaseTypeMismatch);
-    CHECK(diags[4].code == diag::CaseOutsideRange);
-    CHECK(diags[5].code == diag::CaseTypeMismatch);
+    CHECK(diags[4].code == diag::CaseOverlap);
+    CHECK(diags[5].code == diag::CaseOutsideRange);
     CHECK(diags[6].code == diag::CaseTypeMismatch);
+    CHECK(diags[7].code == diag::CaseTypeMismatch);
 }
 
 TEST_CASE("Case statement missing enum values") {
@@ -1210,3 +1211,40 @@ endmodule
     CHECK(diags[0].code == diag::CaseDup);
     CHECK(diags[1].code == diag::CaseDup);
 }
+
+TEST_CASE("Case statement overlap") {
+    auto tree = SyntaxTree::fromText(R"(
+module m;
+    logic [65:0] a;
+    initial begin
+        casex (a)
+            65'bx1011z:;
+            65'b10111:;
+            65'b11011:;
+            65'b1101x:;
+            65'b110?1:;
+            default;
+        endcase
+        casez (a)
+            65'bx1111:;
+            65'b1x111:;
+            65'b1?111:;
+            65'b?1111:;
+            65'b?0111:;
+            default;
+        endcase
+    end
+endmodule
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+
+    auto& diags = compilation.getAllDiagnostics();
+    REQUIRE(diags.size() == 5);
+    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);
+}