Change the default behaviour to raise a violation if parameter's are …

…declared in packages. The use of localparams in packages is prefered and is more in line wit the LRM (System Verilog LRM IEEE Std 1800™-2017, Section 6.20.4)
chipsalliance · Jun 11, 2024 · 7093960 · 7093960
1 parent 48a03f3
commit 7093960
Show file tree

Hide file tree

Showing 4 changed files with 9 additions and 9 deletions.
diff --git a/verilog/analysis/checkers/proper_parameter_declaration_rule.cc b/verilog/analysis/checkers/proper_parameter_declaration_rule.cc
@@ -70,13 +70,13 @@ const LintRuleDescriptor &ProperParameterDeclarationRule::GetDescriptor() {
       .param = {
           {
               .name = "package_allow_parameter",
-              .default_value = "true",
+              .default_value = "false",
               .description = "Allow parameters in packages (treated as a "
                              "synonym for localparam).",
           },
           {
               .name = "package_allow_localparam",
-              .default_value = "false",
+              .default_value = "true",
               .description = "Allow localparams in packages.",
           },
       }};

diff --git a/verilog/analysis/checkers/proper_parameter_declaration_rule.h b/verilog/analysis/checkers/proper_parameter_declaration_rule.h
@@ -47,8 +47,8 @@ class ProperParameterDeclarationRule : public verible::SyntaxTreeLintRule {
  private:
   std::set<verible::LintViolation> violations_;
 
-  bool package_allow_parameter_ = true;
-  bool package_allow_localparam_ = false;
+  bool package_allow_parameter_ = false;
+  bool package_allow_localparam_ = true;
 };
 
 }  // namespace analysis

diff --git a/verilog/analysis/checkers/proper_parameter_declaration_rule_test.cc b/verilog/analysis/checkers/proper_parameter_declaration_rule_test.cc
@@ -89,9 +89,7 @@ TEST(ProperParameterDeclarationRuleTest, RejectPackageParameters) {
        "endmodule"},
   };
 
-  RunConfiguredLintTestCases<VerilogAnalyzer, ProperParameterDeclarationRule>(
-      kTestCases,
-      "package_allow_parameter:false;package_allow_localparam:true");
+  RunLintTestCases<VerilogAnalyzer, ProperParameterDeclarationRule>(kTestCases);
 }
 
 // Tests that the expected number of localparam usage violations are found.
@@ -221,7 +219,9 @@ TEST(ProperParameterDeclarationRuleTest, AllowPackageParameters) {
        "endpackage"},
   };
 
-  RunLintTestCases<VerilogAnalyzer, ProperParameterDeclarationRule>(kTestCases);
+  RunConfiguredLintTestCases<VerilogAnalyzer, ProperParameterDeclarationRule>(
+      kTestCases,
+      "package_allow_parameter:true;package_allow_localparam:false");
 }
 
 }  // namespace

diff --git a/verilog/tools/lint/testdata/endif_comment.sv b/verilog/tools/lint/testdata/endif_comment.sv
@@ -1,5 +1,5 @@
 package endif_comment;
 `ifdef FOOBAR
-  parameter int P = 4;
+  localparam int P = 4;
 `endif
 endpackage