Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added an always_wrap_module_instantiations flag #1909

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions verilog/formatting/format_style.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
5 changes: 5 additions & 0 deletions verilog/formatting/format_style_init.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down Expand Up @@ -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
}
Expand Down
43 changes: 43 additions & 0 deletions verilog/formatting/formatter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15438,6 +15438,49 @@ 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 << "<EOF>";
std::ostringstream stream;
const auto status =
FormatVerilog(test_case.input, "<filename>", 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[] = {
{"", ""},
Expand Down
12 changes: 11 additions & 1 deletion verilog/formatting/tree_unwrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -862,6 +862,16 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions(
VisitIndentedSection(node, 0, PartitionPolicyEnum::kFitOnLineElseExpand);
break;
}
case NodeEnum::kDataDeclaration: {
const auto policy =
(style_.always_wrap_module_instantiations &&
(GetParamListFromDataDeclaration(node) ||
!SearchSyntaxTree(node, NodekPortActualList()).empty()))
? PartitionPolicyEnum::kAlwaysExpand
: PartitionPolicyEnum::kFitOnLineElseExpand;
VisitIndentedSection(node, 0, policy);
break;
}

// The following cases will always expand into their constituent
// partitions:
Expand Down
2 changes: 2 additions & 0 deletions verilog/tools/formatter/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ To pipe from stdin, use '-' as <file>.
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:
Expand Down