Merge origin/master into fix/886 Resolve the issue-regression conflict by placing the #886 tests before the end/else-if regression, away from other open PR tests.
diff --git a/verible/common/formatting/token-partition-tree.cc b/verible/common/formatting/token-partition-tree.cc index 183fee5..7a296b1 100644 --- a/verible/common/formatting/token-partition-tree.cc +++ b/verible/common/formatting/token-partition-tree.cc
@@ -783,6 +783,39 @@ // // 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) { + const auto &children = node.Children(); + return std::count_if( + children.begin(), children.end(), + [](const TokenPartitionTree &child) { return !is_leaf(child); }); +} + +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 +827,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];
diff --git a/verible/verilog/formatting/formatter_issue_regression_test.cc b/verible/verilog/formatting/formatter_issue_regression_test.cc index 9490a88..9b3bb7d 100644 --- a/verible/verilog/formatting/formatter_issue_regression_test.cc +++ b/verible/verilog/formatting/formatter_issue_regression_test.cc
@@ -135,6 +135,109 @@ } } +// 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) + RunFormatterTestCases(style, kTestCases); +} + +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"}, + }; + RunFormatterTestCases40(kTestCases); +} + // Regression for https://github.com/chipsalliance/verible/issues/2540: // Trailing EOL comment after `end` before `else if` must not change whether // the else-if assignment stays on one line across re-format (convergence).