PR #224: Fix Interface and module port list formatting

Fixes #25 #218 GitHub PR #224 Copybara import of the project: - 73f66fd Expand port and parameter lists in module and interface d... by Lukasz Dalek <[email protected]> - 552732b Put kPort in its own partition by Lukasz Dalek <[email protected]> - 0f1cbde Update module and interface declarations tests by Lukasz Dalek <[email protected]> Closes #224 PiperOrigin-RevId: 298909899
chipsalliance · Mar 4, 2020 · 55adfae · 55adfae
1 parent 9f748a8
commit 55adfae
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 17 deletions.
diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc
@@ -632,26 +632,38 @@ static const std::initializer_list<FormatterTestCase> kFormatterTestCases = {
      ");\n"
      "endmodule\n"},
     {"module foo(  input x  , output y ) ;endmodule:foo\n",
-     "module foo (input x, output y);\n"  // entire header fits on one line
+     "module foo (\n"
+     "    input x,\n"
+     "    output y\n"
+     ");\n"  // entire header fits on one line
      "endmodule : foo\n"},
     {"module foo(  input[2:0]x  , output y [3:0] ) ;endmodule:foo\n",
      // TODO(fangism): reduce spaces around ':' in dimensions
+     // each port item should be on its own line
      "module foo (\n"
-     "    input [2:0] x, output y[3:0]\n"
-     // module header and port list fits on one line
+     "    input [2:0] x,\n"
+     "    output y[3:0]\n"
      ");\n"
      "endmodule : foo\n"},
     {"module foo #(int x,int y) ;endmodule:foo\n",  // parameters
-     "module foo #(int x, int y);\n"  // entire header fits on one line
+     "module foo #(\n"
+     "    int x,\n"
+     "    int y\n"
+     ");\n"  // each paramater on its own line
      "endmodule : foo\n"},
     {"module foo #(int x)(input y) ;endmodule:foo\n",
      // parameter and port
-     "module foo #(int x) (input y);\n"  // entire header fits on one line
+     "module foo #(\n"
+     "    int x\n"
+     ") (\n"
+     "    input y\n"
+     ");\n"  // each paramater and port item should be on its own line
      "endmodule : foo\n"},
     {"module foo #(parameter int x,parameter int y) ;endmodule:foo\n",
-     // parameters don't fit
+     // parameters don't fit (also should be on its own line)
      "module foo #(\n"
-     "    parameter int x, parameter int y\n"
+     "    parameter int x,\n"
+     "    parameter int y\n"
      ");\n"
      "endmodule : foo\n"},
     {"module foo #(parameter int xxxx,parameter int yyyy) ;endmodule:foo\n",
@@ -1352,15 +1364,19 @@ static const std::initializer_list<FormatterTestCase> kFormatterTestCases = {
      "endinterface : if2\n"},
     {// interface declaration with parameters
      " interface if1#( parameter int W= 8 );endinterface\t\t",
-     "interface if1 #(parameter int W = 8);\n"
+     "interface if1 #(\n"
+     "    parameter int W = 8\n"
+     ");\n"
      "endinterface\n"},
     {// interface declaration with ports (empty)
      " interface if1()\n;endinterface\t\t",
      "interface if1 ();\n"
      "endinterface\n"},
     {// interface declaration with ports
      " interface if1( input\tlogic   z)\n;endinterface\t\t",
-     "interface if1 (input logic z);\n"
+     "interface if1 (\n"
+     "    input logic z\n"
+     ");\n"
      "endinterface\n"},
     {// interface declaration with parameters and ports
      " interface if1#( parameter int W= 8 )(input logic z);endinterface\t\t",
@@ -2365,11 +2381,16 @@ static const std::initializer_list<FormatterTestCase> kFormatterTestCases = {
      "module foo ();\n"
      "endmodule\n"},
     {"   module       foo   (  .x (  x) );     endmodule\n",
-     "module foo (.x(x));\n"
+     "module foo (\n"
+     "    .x(x)\n"
+     ");\n"
      "endmodule\n"},
     {"   module       foo   (  .x (  x)  \n,\n . y "
      "  ( \ny) );     endmodule\n",
-     "module foo (.x(x), .y(y));\n"
+     "module foo (\n"
+     "    .x(x),\n"
+     "    .y(y)\n"
+     ");\n"
      "endmodule\n"},
 
     // module instantiation test cases

diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc
@@ -601,6 +601,7 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions(
     case NodeEnum::kActualNamedPort:
     case NodeEnum::kActualPositionalPort:
     case NodeEnum::kAssertionVariableDeclaration:
+    case NodeEnum::kPort:
     case NodeEnum::kPortItem:
     case NodeEnum::kPropertyDeclaration:
     case NodeEnum::kSequenceDeclaration:
@@ -931,8 +932,7 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions(
     case NodeEnum::kVariableDeclarationAssignmentList:
     case NodeEnum::kPortActualList:  // TODO(b/146083526): one port per line
     case NodeEnum::kActualParameterByNameList:
-    case NodeEnum::kPortList:
-    case NodeEnum::kPortDeclarationList: {
+    case NodeEnum::kPortList: {
       if (suppress_indentation) {
         // Do not further indent preprocessor clauses.
         // Maintain same level as before.
@@ -944,6 +944,7 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions(
       break;
     }
 
+    case NodeEnum::kPortDeclarationList:
     case NodeEnum::kFormalParameterList: {
       if (suppress_indentation) {
         // Do not further indent preprocessor clauses.
@@ -952,6 +953,10 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions(
       } else if (Context().IsInside(NodeEnum::kClassHeader)) {
         VisitIndentedSection(node, style_.wrap_spaces,
                              PartitionPolicyEnum::kAlwaysExpand);
+      } else if (Context().IsInside(NodeEnum::kInterfaceDeclaration) ||
+                 Context().IsInside(NodeEnum::kModuleDeclaration)) {
+        VisitIndentedSection(node, style_.wrap_spaces,
+                             PartitionPolicyEnum::kAlwaysExpand);
       } else {
         VisitIndentedSection(node, style_.wrap_spaces,
                              PartitionPolicyEnum::kFitOnLineElseExpand);

diff --git a/verilog/formatting/tree_unwrapper_test.cc b/verilog/formatting/tree_unwrapper_test.cc
@@ -506,10 +506,12 @@ const TreeUnwrapperTestData kUnwrapModuleTestCases[] = {
         "  co = (a&b);"
         "end "
         "endmodule",
-        ModuleHeader(0, L(0, {"module", "addf", "("}),
-                     ModulePortList(2, L(2, {"a", ",", "b", ",", "ci", ",", "s",
-                                             ",", "co"})),
-                     L(0, {")", ";"})),
+        ModuleHeader(
+            0, L(0, {"module", "addf", "("}),
+            ModulePortList(2,  //
+                           L(2, {"a", ","}), L(2, {"b", ","}),
+                           L(2, {"ci", ","}), L(2, {"s", ","}), L(2, {"co"})),
+            L(0, {")", ";"})),
         ModuleItemList(
             1, L(1, {"input", "a", ",", "b", ",", "ci", ";"}),
             L(1, {"output", "s", ",", "co", ";"}),