From 71baff5b38955f57fe5ce60aea2557bacd1d7049 Mon Sep 17 00:00:00 2001 From: Jan Bylicki Date: Fri, 5 May 2023 11:13:48 +0200 Subject: [PATCH 1/6] format_style: Added the always_wrap_module_instantiations flag Signed-off-by: Jan Bylicki --- verilog/formatting/format_style.h | 3 +++ verilog/formatting/format_style_init.cc | 5 +++++ verilog/formatting/tree_unwrapper.cc | 8 +++++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/verilog/formatting/format_style.h b/verilog/formatting/format_style.h index a9a020ba7..b4eaf0288 100644 --- a/verilog/formatting/format_style.h +++ b/verilog/formatting/format_style.h @@ -116,6 +116,9 @@ struct FormatStyle : public verible::BasicFormatStyle { // Split with a \n end and else clauses bool wrap_end_else_clauses = false; + // Always split module instantiation's parameters and ports to new lines + bool always_wrap_module_instantiations = false; + // -- Note: when adding new fields, add them in format_style_init.cc // TODO(fangism): introduce the following knobs: diff --git a/verilog/formatting/format_style_init.cc b/verilog/formatting/format_style_init.cc index 38c58dcae..e9afa72ee 100644 --- a/verilog/formatting/format_style_init.cc +++ b/verilog/formatting/format_style_init.cc @@ -88,6 +88,10 @@ ABSL_FLAG(bool, compact_indexing_and_selections, true, ABSL_FLAG(bool, wrap_end_else_clauses, false, "Split end and else keywords into separate lines"); +ABSL_FLAG( + bool, always_wrap_module_instantiations, false, + "Always split module instantiation's parameters and ports to new lines"); + ABSL_FLAG(bool, port_declarations_right_align_packed_dimensions, false, "If true, packed dimensions in contexts with enabled alignment are " "aligned to the right."); @@ -136,6 +140,7 @@ void InitializeFromFlags(FormatStyle *style) { STYLE_FROM_FLAG(expand_coverpoints); STYLE_FROM_FLAG(compact_indexing_and_selections); STYLE_FROM_FLAG(wrap_end_else_clauses); + STYLE_FROM_FLAG(always_wrap_module_instantiations); #undef STYLE_FROM_FLAG } diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc index 346542985..558f7b3b4 100644 --- a/verilog/formatting/tree_unwrapper.cc +++ b/verilog/formatting/tree_unwrapper.cc @@ -50,6 +50,7 @@ #include "verilog/CST/functions.h" #include "verilog/CST/macro.h" #include "verilog/CST/statement.h" +#include "verilog/CST/verilog_matchers.h" #include "verilog/CST/verilog_nonterminals.h" #include "verilog/formatting/verilog_token.h" #include "verilog/parser/verilog_parser.h" @@ -834,7 +835,6 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions( case NodeEnum::kMacroGenericItem: case NodeEnum::kModuleHeader: case NodeEnum::kBindDirective: - case NodeEnum::kDataDeclaration: case NodeEnum::kGateInstantiation: case NodeEnum::kLoopHeader: case NodeEnum::kCovergroupHeader: @@ -862,6 +862,12 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions( VisitIndentedSection(node, 0, PartitionPolicyEnum::kFitOnLineElseExpand); break; } + case NodeEnum::kDataDeclaration: { + VisitIndentedSection(node, 0, + PartitionPolicyEnum::kFitOnLineElseExpand); + } + break; + } // The following cases will always expand into their constituent // partitions: From 8d48a0477a8499287d2d786965b80261432e3acc Mon Sep 17 00:00:00 2001 From: Jan Bylicki Date: Fri, 5 May 2023 11:14:50 +0200 Subject: [PATCH 2/6] tree_unwrapper: Added kDataDeclaration formatting logic Signed-off-by: Jan Bylicki --- verilog/formatting/tree_unwrapper.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc index 558f7b3b4..a6d293428 100644 --- a/verilog/formatting/tree_unwrapper.cc +++ b/verilog/formatting/tree_unwrapper.cc @@ -863,6 +863,11 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions( break; } case NodeEnum::kDataDeclaration: { + if ((GetParamListFromDataDeclaration(node) || + !SearchSyntaxTree(node, NodekPortActualList()).empty()) && + style_.always_wrap_module_instantiations) { + VisitIndentedSection(node, 0, PartitionPolicyEnum::kAlwaysExpand); + } else { VisitIndentedSection(node, 0, PartitionPolicyEnum::kFitOnLineElseExpand); } From a5216a1104bafd27354b969a391af80d10ea784c Mon Sep 17 00:00:00 2001 From: Jan Bylicki Date: Fri, 5 May 2023 11:15:17 +0200 Subject: [PATCH 3/6] formatter_test: Added end-to-end always_wrap_module_instantiations tests Signed-off-by: Jan Bylicki --- verilog/formatting/formatter_test.cc | 37 ++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc index 89883077c..83b636fa6 100644 --- a/verilog/formatting/formatter_test.cc +++ b/verilog/formatting/formatter_test.cc @@ -15438,6 +15438,43 @@ TEST(FormatterEndToEndTest, VerilogFormatTest) { } } +TEST(FormatterEndToEndTest, AlwaysWrapModuleInstantiation) { + static constexpr FormatterTestCase kTestCases[] = { + {" module foo ; bar bq();endmodule\n", + "module foo;\n" + " bar bq ();\n" // single instance + "endmodule\n"}, + {" module foo ; bar bq(), bq2( );endmodule\n", + "module foo;\n" + " bar bq (), bq2 ();\n" // multiple empty instances, still fitting on + // one line + "endmodule\n"}, + {"module foo; bar #(.N(N)) bq (.bus(bus));endmodule\n", + // instance parameter and port fits on line + "module foo;\n" + " bar #(\n .N(N)\n ) bq (\n .bus(bus)\n );\n" + "endmodule\n"}, + {"module foo; bar bq (.bus(bus));endmodule\n", + "module foo;\n" + " bar bq (\n .bus(bus)\n );\n" + "endmodule\n"}, + }; + FormatStyle style; + style.column_limit = 40; + style.indentation_spaces = 2; + style.wrap_spaces = 4; + style.always_wrap_module_instantiations = true; + for (const auto& test_case : kTestCases) { + VLOG(1) << "code-to-format:\n" << test_case.input << ""; + std::ostringstream stream; + const auto status = + FormatVerilog(test_case.input, "", style, stream); + // Require these test cases to be valid. + EXPECT_OK(status) << status.message(); + EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input; + } +} + TEST(FormatterEndToEndTest, AutoInferAlignment) { static constexpr FormatterTestCase kTestCases[] = { {"", ""}, From 8b72f4500a875616d73d4361e10e92a91997ac0b Mon Sep 17 00:00:00 2001 From: Jan Bylicki Date: Tue, 23 May 2023 17:41:36 +0200 Subject: [PATCH 4/6] formatter: README.md: Updated usage section Signed-off-by: Jan Bylicki --- verilog/tools/formatter/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/verilog/tools/formatter/README.md b/verilog/tools/formatter/README.md index f0acb3112..29075d5b0 100644 --- a/verilog/tools/formatter/README.md +++ b/verilog/tools/formatter/README.md @@ -32,6 +32,8 @@ To pipe from stdin, use '-' as . operator.); default: 4; Flags from verilog/formatting/format_style_init.cc: + --always_wrap_module_instantiations (Always split module instantiation's + parameters and ports to new lines); default: false; --assignment_statement_alignment (Format various assignments: {align,flush-left,preserve,infer}); default: infer; --case_items_alignment (Format case items: From 47af18c5300262cd53126460da3575556c4f8f25 Mon Sep 17 00:00:00 2001 From: Jan Bylicki <47863101+jbylicki@users.noreply.github.com> Date: Tue, 27 Jun 2023 12:21:47 +0200 Subject: [PATCH 5/6] tree_unwrapper.cc: Optimized policy search condition checks Co-authored-by: Mariusz Glebocki Signed-off-by: Jan Bylicki --- verilog/formatting/tree_unwrapper.cc | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc index a6d293428..b8f8f5cf1 100644 --- a/verilog/formatting/tree_unwrapper.cc +++ b/verilog/formatting/tree_unwrapper.cc @@ -863,14 +863,13 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions( break; } case NodeEnum::kDataDeclaration: { - if ((GetParamListFromDataDeclaration(node) || - !SearchSyntaxTree(node, NodekPortActualList()).empty()) && - style_.always_wrap_module_instantiations) { - VisitIndentedSection(node, 0, PartitionPolicyEnum::kAlwaysExpand); - } else { - VisitIndentedSection(node, 0, - PartitionPolicyEnum::kFitOnLineElseExpand); - } + const auto policy = + (style_.always_wrap_module_instantiations && + (GetParamListFromDataDeclaration(node) || + !SearchSyntaxTree(node, NodekPortActualList()).empty())) + ? PartitionPolicyEnum::kAlwaysExpand + : PartitionPolicyEnum::kFitOnLineElseExpand; + VisitIndentedSection(node, 0, policy); break; } From 8f33d2d195af23b3969f0f3824f4da1e46495e2d Mon Sep 17 00:00:00 2001 From: Jan Bylicki Date: Tue, 27 Jun 2023 12:24:37 +0200 Subject: [PATCH 6/6] formatter_test: Changed test styling to break newlines in strings Signed-off-by: Jan Bylicki --- verilog/formatting/formatter_test.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc index 83b636fa6..c40d44f5b 100644 --- a/verilog/formatting/formatter_test.cc +++ b/verilog/formatting/formatter_test.cc @@ -15452,11 +15452,17 @@ TEST(FormatterEndToEndTest, AlwaysWrapModuleInstantiation) { {"module foo; bar #(.N(N)) bq (.bus(bus));endmodule\n", // instance parameter and port fits on line "module foo;\n" - " bar #(\n .N(N)\n ) bq (\n .bus(bus)\n );\n" + " bar #(\n" + " .N(N)\n" + " ) bq (\n" + " .bus(bus)\n" + " );\n" "endmodule\n"}, {"module foo; bar bq (.bus(bus));endmodule\n", "module foo;\n" - " bar bq (\n .bus(bus)\n );\n" + " bar bq (\n" + " .bus(bus)\n" + " );\n" "endmodule\n"}, }; FormatStyle style;