linter: do not require 'default' case for 'unique case'

case-missing-default rule required the 'default' case for every case statement. This commit changes it so that case statements with the 'unique' qualifier are not marked as errors by this check.
chipsalliance · Aug 15, 2023 · ab245b5 · ab245b5
1 parent 0b8cb4e
commit ab245b5
Show file tree

Hide file tree

Showing 4 changed files with 149 additions and 76 deletions.
diff --git a/verilog/CST/verilog_matchers.h b/verilog/CST/verilog_matchers.h
@@ -1,4 +1,4 @@
-// Copyright 2017-2020 The Verible Authors.
+// Copyright 2017-2023 The Verible Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -411,8 +411,32 @@ inline constexpr auto DeclarationDimensionsHasRanges =
 //   endcase
 //
 inline constexpr auto HasDefaultCase =
-    verible::matcher::MakePathMatcher(N(kDefaultItem));
+    verible::matcher::MakePathMatcher(N(kCaseItemList), N(kDefaultItem));
 
+// Matches with statements qualified with "unique"
+// For instance, matches:
+//
+//   unique case (in)
+//     default: return 0;
+//   endcase
+//
+//   unique if (a)
+//     ...
+//   else if (!a)
+//     ...
+//
+// but not:
+//
+//   case (in)
+//     default: return 0;
+//   endcase
+//
+//   if (a)
+//     ...
+//   else if (!a)
+//     ...
+inline constexpr auto HasUniqueQualifier =
+    verible::matcher::MakePathMatcher(L(TK_unique));
 // Clean up macros
 #undef N
 #undef L

diff --git a/verilog/CST/verilog_matchers_test.cc b/verilog/CST/verilog_matchers_test.cc
@@ -1,4 +1,4 @@
-// Copyright 2017-2020 The Verible Authors.
+// Copyright 2017-2023 The Verible Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -789,5 +789,44 @@ TEST(VerilogMatchers, HasDefaultCaseTests) {
   }
 }
 
+// Tests for HasUniqueQualifier matching.
+TEST(VerilogMatchers, HasUniqueQualifierTests) {
+  const RawMatcherTestCase tests[] = {
+      {HasUniqueQualifier(), "", 0},
+      {HasUniqueQualifier(),
+       R"(
+       function automatic int foo (input in);
+         case (in)
+           default: return 0;
+         endcase
+       endfunction
+       )",
+       0},
+      {HasUniqueQualifier(),
+       R"(
+       function automatic int foo (input in);
+         unique case (in)
+           1: return 0;
+         endcase
+       endfunction
+       )",
+       1},
+      {HasUniqueQualifier(),
+       R"(
+       function automatic int foo (input in);
+         unique if (in) begin
+           return 0;
+         end
+         else if(!in) begin
+           return 1;
+         end
+       endfunction
+       )",
+       1},
+  };
+  for (const auto &test : tests) {
+    verible::matcher::RunRawMatcherTestCase<VerilogAnalyzer>(test);
+  }
+}
 }  // namespace
 }  // namespace verilog
diff --git a/verilog/analysis/checkers/case_missing_default_rule.cc b/verilog/analysis/checkers/case_missing_default_rule.cc
@@ -1,4 +1,4 @@
-// Copyright 2017-2020 The Verible Authors.
+// Copyright 2017-2023 The Verible Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -40,28 +40,39 @@ using verible::matcher::Matcher;
 VERILOG_REGISTER_LINT_RULE(CaseMissingDefaultRule);
 
 static constexpr absl::string_view kMessage =
-    "Explicitly define a default case for every case statement.";
+    "Explicitly define a default case for every case statement or add `unique` "
+    "qualifier to the case statement.";
 
 const LintRuleDescriptor &CaseMissingDefaultRule::GetDescriptor() {
   static const LintRuleDescriptor d{
       .name = "case-missing-default",
       .topic = "case-statements",
-      .desc = "Checks that a default case-item is always defined.",
+      .desc =
+          "Checks that a default case-item is always defined unless the case "
+          "statement has the `unique` qualifier.",
   };
   return d;
 }
 
-static const Matcher &CaseMatcher() {
-  static const Matcher matcher(
-      NodekCaseItemList(verible::matcher::Unless(HasDefaultCase())));
-  return matcher;
-}
-
 void CaseMissingDefaultRule::HandleSymbol(
     const verible::Symbol &symbol, const verible::SyntaxTreeContext &context) {
   verible::matcher::BoundSymbolManager manager;
-  if (context.DirectParentIs(NodeEnum::kCaseStatement) &&
-      CaseMatcher().Matches(symbol, &manager)) {
+
+  // Ensure that symbol is a kCaseStatement node
+  if (symbol.Kind() != verible::SymbolKind::kNode) return;
+  const verible::SyntaxTreeNode &node = verible::SymbolCastToNode(symbol);
+  if (!node.MatchesTag(NodeEnum::kCaseStatement)) return;
+
+  static const Matcher uniqueCaseMatcher(
+      NodekCaseStatement(HasUniqueQualifier()));
+
+  static const Matcher caseMatcherWithDefaultCase(
+      NodekCaseStatement(HasDefaultCase()));
+
+  // If the case statement doesn't have the "unique" qualifier and
+  // it is missing the "default" case, insert the violation
+  if (!uniqueCaseMatcher.Matches(symbol, &manager) &&
+      !caseMatcherWithDefaultCase.Matches(symbol, &manager)) {
     violations_.insert(LintViolation(symbol, kMessage, context));
   }
 }

diff --git a/verilog/analysis/checkers/case_missing_default_rule_test.cc b/verilog/analysis/checkers/case_missing_default_rule_test.cc
@@ -1,4 +1,4 @@
-// Copyright 2017-2020 The Verible Authors.
+// Copyright 2017-2023 The Verible Authors.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -44,7 +44,6 @@ TEST(CaseMissingDefaultRuleTest, BasicTests) {
 // Tests that the expected number of case-missing-default-rule violations inside
 // functions are found.
 TEST(CaseMissingDefaultRuleTest, CaseInsideFunctionTests) {
-  // Currently, diagnostics point to the leftmost token of the kCaseItemList.
   const std::initializer_list<LintTestCase> kTestCases = {
       // case tests
       {R"(
@@ -56,10 +55,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideFunctionTests) {
        )"},
       {R"(
        function automatic int foo (input in);
-         case (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: return 0;
+         )",
+       {TK_case, "case"},
+       R"( (in)
+           1: return 0;
          endcase
        endfunction
        )"},
@@ -74,10 +73,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideFunctionTests) {
        )"},
       {R"(
        function automatic int foo (input in);
-         casex (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: return 0;
+         )",
+       {TK_casex, "casex"},
+       R"( (in)
+           1: return 0;
          endcase
        endfunction
        )"},
@@ -92,10 +91,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideFunctionTests) {
        )"},
       {R"(
        function automatic int foo (input in);
-         casez (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: return 0;
+         )",
+       {TK_casez, "casez"},
+       R"( (in)
+           1: return 0;
          endcase
        endfunction
        )"},
@@ -129,10 +128,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideTaskTests) {
        )"},
       {R"(
        task automatic foo (input in);
-         case (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: return 0;
+         )",
+       {TK_case, "case"},
+       R"( (in)
+           1: return 0;
          endcase
        endtask
        )"},
@@ -147,10 +146,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideTaskTests) {
        )"},
       {R"(
        task automatic foo (input in);
-         casex (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: return 0;
+         )",
+       {TK_casex, "casex"},
+       R"( (in)
+           1: return 0;
          endcase
        endtask
        )"},
@@ -165,10 +164,10 @@ TEST(CaseMissingDefaultRuleTest, CaseInsideTaskTests) {
        )"},
       {R"(
        task automatic foo (input in);
-         casez (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: return 0;
+         )",
+       {TK_casez, "casez"},
+       R"( (in)
+           1: return 0;
          endcase
        endtask
        )"},
@@ -194,13 +193,13 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideFunctionTests) {
   const std::initializer_list<LintTestCase> kTestCases = {
       // Inner case doesn't have default, outer case does.
       {R"(
-       function automatic int foo (input in);
+       function automatic foo (input in);
          case (in)
            1: begin;
-             case (in)
-               )",
-       {TK_DecNumber, "1"},
-       R"(: return 1;
+             )",
+       {TK_case, "case"},
+       R"( (in)
+               1: return 1;
              endcase
            end
            default: return 1;
@@ -210,11 +209,11 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideFunctionTests) {
 
       // Inner case does have default, outer case doesn't.
       {R"(
-       function automatic int foo (input in);
-         case (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: begin;
+       function automatic foo (input in);
+         )",
+       {TK_case, "case"},
+       R"( (in)
+           1: begin;
              case (in)
                1: return 1;
                default: return 1;
@@ -226,7 +225,7 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideFunctionTests) {
 
       // Both inner and outer case have default case statements.
       {R"(
-       function automatic int foo (input in);
+       function automatic foo (input in);
          case (in)
            1: begin;
              case (in)
@@ -241,15 +240,15 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideFunctionTests) {
 
       // Neither of the cases have default case statements.
       {R"(
-       function automatic int foo (input in);
-         case (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: begin;
-             case (in)
-               )",
-       {TK_DecNumber, "1"},
-       R"(: return 1;
+       function automatic foo (input in);
+         )",
+       {TK_case, "case"},
+       R"( (in)
+           1: begin;
+             )",
+       {TK_case, "case"},
+       R"( (in)
+               1: return 1;
              endcase
            end
          endcase
@@ -269,10 +268,10 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideTaskTests) {
        task automatic foo (input in);
          case (in)
            1: begin;
-             case (in)
-               )",
-       {TK_DecNumber, "1"},
-       R"(: return 1;
+             )",
+       {TK_case, "case"},
+       R"( (in)
+               1: return 1;
              endcase
            end
            default: return 1;
@@ -283,10 +282,10 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideTaskTests) {
       // Inner case does have default, outer case doesn't.
       {R"(
        task automatic foo (input in);
-         case (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: begin;
+         )",
+       {TK_case, "case"},
+       R"( (in)
+           1: begin;
              case (in)
                1: return 1;
                default: return 1;
@@ -314,14 +313,14 @@ TEST(CaseMissingDefaultRuleTest, NestedCaseInsideTaskTests) {
       // Neither of the cases have default case statements.
       {R"(
        task automatic foo (input in);
-         case (in)
-           )",
-       {TK_DecNumber, "1"},
-       R"(: begin;
-             case (in)
-               )",
-       {TK_DecNumber, "1"},
-       R"(: return 1;
+         )",
+       {TK_case, "case"},
+       R"( (in)
+           1: begin;
+             )",
+       {TK_case, "case"},
+       R"( (in)
+               1: return 1;
              endcase
            end
          endcase