Keep function ports when packed dims contain $clog2()

ReshapeFittingSubpartitions assumed [header, args, trailer]. Extra
header fragments from $clog2() in packed dimensions caused the port
list to be dropped (issue #886).

verible/common/formatting/token-partition-tree.cc

diff --git a/verible/common/formatting/token-partition-tree.cc b/verible/common/formatting/token-partition-tree.cc
index 183fee5..6f3c556 100644
--- a/verible/common/formatting/token-partition-tree.cc
+++ b/verible/common/formatting/token-partition-tree.cc
@@ -783,6 +783,41 @@
 //
 // When "subpartitions" group has kAlwaysExpand policy, line break is forced
 // between each subpartition from the group.
+// kAppendFittingSubPartitions expects [header, args] or [header, args,
+// trailer]. Packed dimensions with $clog2(...) (issue #886) can split the
+// header into extra sibling leaves; if those are left in place, the port
+// list is treated as a trailer and dropped.
+// Only merge leading *leaf* fragments. Nested argument lists (non-leaves)
+// are flattened only when another non-leaf (the real port list) follows.
+static int CountNonLeafChildren(const TokenPartitionTree &node) {
+  int n = 0;
+  for (const auto &child : node.Children()) {
+    if (!is_leaf(child)) ++n;
+  }
+  return n;
+}
+
+static void CollapseHeaderFragmentsBeforeArgs(TokenPartitionTree *node) {
+  while (node->Children().size() > 2) {
+    auto &children = node->Children();
+    auto &first = children[0];
+    auto &second = children[1];
+    // Merge extra header leaves only when a nested argument list still
+    // follows.  All-leaf trees are the flattened one-argument form
+    // ([header, arg] or [header, arg, trailer]) and must be left intact.
+    if (is_leaf(first) && is_leaf(second) &&
+        CountNonLeafChildren(*node) >= 1) {
+      MergeConsecutiveSiblings(node, 0);
+      continue;
+    }
+    if (!is_leaf(second) && CountNonLeafChildren(*node) >= 2) {
+      FlattenOneChild(*node, 1);
+      continue;
+    }
+    break;
+  }
+}
+
 void ReshapeFittingSubpartitions(const BasicFormatStyle &style,
                                  TokenPartitionTree *node) {
   VLOG(4) << __FUNCTION__ << ", before:\n" << *node;
@@ -794,6 +829,11 @@
     return;
   }
 
+  CollapseHeaderFragmentsBeforeArgs(node);
+  if (node->Children().size() < 2) {
+    return;
+  }
+
   // Partition with arguments should have at least one argument
   const auto &children = node->Children();
   const auto &header = children[0];

verible/verilog/formatting/formatter_test.cc

diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc
index dce9f05..07dd4bb 100644
--- a/verible/verilog/formatting/formatter_test.cc
+++ b/verible/verilog/formatting/formatter_test.cc
@@ -20959,6 +20959,122 @@
   }
 }
 
+// Regression for https://github.com/chipsalliance/verible/issues/886:
+// Packed dimensions with $clog2()/$bits() used to split the function header
+// so ReshapeFittingSubpartitions dropped the port list.
+TEST(FormatterEndToEndTest, FunctionHeaderPackedDimSystemCallKeepsPorts) {
+  static constexpr FormatterTestCase kTestCases[] = {
+      {// Original issue sample (default column_limit 100)
+       "package foo;\n"
+       "  function some_large_return_type [$clog2(some_large_contant_name)-1:0] "
+       "f_some_long_function( input int parameter_1, input int parameter_2);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n",
+       "package foo;\n"
+       "  function some_large_return_type [$clog2(some_large_contant_name)-1:0] "
+       "f_some_long_function(\n"
+       "      input int parameter_1, input int parameter_2);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n"},
+      {// Short names still keep ports and stay on one line
+       "package foo;\n"
+       "  function logic [$clog2(N)-1:0] f(input int a, input int b);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n",
+       "package foo;\n"
+       "  function logic [$clog2(N)-1:0] f(input int a, input int b);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n"},
+      {// $bits() in packed dimensions
+       "package foo;\n"
+       "  function some_large_return_type [$bits(some_large_contant_name)-1:0] "
+       "f_some_long_function(input int parameter_1, input int parameter_2);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n",
+       "package foo;\n"
+       "  function some_large_return_type [$bits(some_large_contant_name)-1:0] "
+       "f_some_long_function(\n"
+       "      input int parameter_1, input int parameter_2);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n"},
+      {// Multi-argument system function in packed dimensions
+       "package foo;\n"
+       "  function some_large_return_type "
+       "[$clog2(some_large_contant_name, WIDTH)-1:0] "
+       "f_some_long_function(input int parameter_1, input int parameter_2);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n",
+       "package foo;\n"
+       "  function some_large_return_type "
+       "[$clog2(some_large_contant_name, WIDTH)-1:0] "
+       "f_some_long_function(\n"
+       "      input int parameter_1, input int parameter_2);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n"},
+      {// extern prototype
+       "class c;\n"
+       "  extern function some_large_return_type "
+       "[$clog2(some_large_contant_name)-1:0] "
+       "f_some_long_function(input int parameter_1, input int parameter_2);\n"
+       "endclass\n",
+       "class c;\n"
+       "  extern function some_large_return_type "
+       "[$clog2(some_large_contant_name)-1:0] "
+       "f_some_long_function(\n"
+       "      input int parameter_1, input int parameter_2);\n"
+       "endclass\n"},
+  };
+  FormatStyle style;  // default column_limit (100)
+  for (const auto &test_case : kTestCases) {
+    VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>";
+    std::ostringstream stream;
+    const auto status =
+        FormatVerilog(test_case.input, "<filename>", style, stream);
+    EXPECT_OK(status) << status.message();
+    EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+  }
+}
+
+TEST(FormatterEndToEndTest, FunctionHeaderPackedDimSystemCallWrapsArgs) {
+  // Tight column limit still keeps the ports (the original bug dropped them).
+  // The header itself is longer than 40 columns, so it wraps.
+  static constexpr FormatterTestCase kTestCases[] = {
+      {"package foo;\n"
+       "  function some_large_return_type [$clog2(some_large_contant_name)-1:0] "
+       "f_some_long_function( input int parameter_1, input int parameter_2);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n",
+       "package foo;\n"
+       "  function\n"
+       "      some_large_return_type [$clog2(some_large_contant_name)-1\n"
+       "      :0] f_some_long_function(\n"
+       "      input int parameter_1,\n"
+       "      input int parameter_2);\n"
+       "    return 1;\n"
+       "  endfunction\n"
+       "endpackage\n"},
+  };
+  FormatStyle style;
+  style.column_limit = 40;
+  for (const auto &test_case : kTestCases) {
+    VLOG(1) << "code-to-format:\n" << test_case.input << "<EOF>";
+    std::ostringstream stream;
+    const auto status =
+        FormatVerilog(test_case.input, "<filename>", style, stream);
+    EXPECT_OK(status) << status.message();
+    EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+  }
+}
+
 }  // namespace
 }  // namespace formatter
 }  // namespace verilog