Skip to content

Commit

Permalink
Merge pull request #819 from antmicro/align-function-formal-parameters
Browse files Browse the repository at this point in the history
Align parameters in exported/imported function
  • Loading branch information
tgorochowik authored May 28, 2021
2 parents e204d58 + 57e3796 commit e2d7e72
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 19 deletions.
6 changes: 3 additions & 3 deletions verilog/CST/DPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,18 @@ verible::SymbolPtr MakeDPIImport(T0&& keyword, T1&& spec, T2&& property,
// Partial specialization provided as a workaround to passing nullptr
// in positions 3 and 4 (optional symbols). Compiler is not guaranteed
// to deduce to that some paths are not reachble/applicable.
template <typename T0, typename T1, typename T2, typename T3>
template <typename T0, typename T1, typename T2, typename T3, typename T4>
verible::SymbolPtr MakeDPIImport(T0&& keyword, T1&& spec, T2&& property,
std::nullptr_t id, std::nullptr_t equals,
T3&& proto) {
T3&& proto, T4&& semi) {
verible::CheckSymbolAsLeaf(*keyword, verilog_tokentype::TK_import);
verible::CheckSymbolAsLeaf(*spec, verilog_tokentype::TK_StringLiteral);
CHECK(verible::SymbolCastToNode(*proto).MatchesTagAnyOf(
{NodeEnum::kFunctionPrototype, NodeEnum::kTaskPrototype}));
return verible::MakeTaggedNode(
NodeEnum::kDPIImportItem, std::forward<T0>(keyword),
std::forward<T1>(spec), std::forward<T2>(property), id, equals,
std::forward<T3>(proto));
std::forward<T3>(proto), std::forward<T4>(semi));
}

// Find all DPI imports.
Expand Down
12 changes: 8 additions & 4 deletions verilog/CST/DPI_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,24 +86,28 @@ TEST(GetDPIImportPrototypeTest, Various) {
// each of these test cases should match exactly one DPI import
{"module m;\n"
" import \"DPI-C\" ",
{kTag, "function void foo;"},
{kTag, "function void foo"},
";",
"\n"
"endmodule\n"},
{"module m;\n"
" wire w;\n"
" import \"DPI-C\" ",
{kTag, "function void foo;"},
{kTag, "function void foo"},
";",
"\n"
" logic l;\n"
"endmodule\n"},
{"module m;\n"
" import \"DPI-C\" ",
{kTag, "function int add();"},
{kTag, "function int add()"},
";",
"\n"
"endmodule\n"},
{"module m;\n"
" import \"DPI-C\" ",
{kTag, "function int add( input int x , y);"},
{kTag, "function int add( input int x , y)"},
";",
"\n"
"endmodule\n"},
};
Expand Down
6 changes: 5 additions & 1 deletion verilog/formatting/align.cc
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,10 @@ class PortDeclarationColumnSchemaScanner : public ColumnSchemaScanner {
ReserveNewColumn(node, FlushLeft);
break;
case NodeEnum::kUnqualifiedId:
if (Context().DirectParentIs(NodeEnum::kPortDeclaration)) {
if (Context().DirectParentIs(NodeEnum::kPortDeclaration) ||
Context().DirectParentsAre(
{NodeEnum::kDataTypeImplicitBasicIdDimensions,
NodeEnum::kPortItem})) {
ReserveNewColumn(node, FlushLeft);
}
break;
Expand Down Expand Up @@ -1273,6 +1276,7 @@ void TabularAlignTokenPartitions(TokenPartitionTree* partition_ptr,
static const auto* const kAlignHandlers =
new std::map<NodeEnum, AlignSyntaxGroupsFunction>{
{NodeEnum::kPortDeclarationList, &AlignPortDeclarations},
{NodeEnum::kPortList, &AlignPortDeclarations},
{NodeEnum::kActualParameterByNameList, &AlignActualNamedParameters},
{NodeEnum::kPortActualList, &AlignActualNamedPorts},
{NodeEnum::kModuleItemList, &AlignModuleItems},
Expand Down
39 changes: 36 additions & 3 deletions verilog/formatting/formatter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2884,24 +2884,57 @@ static constexpr FormatterTestCase kFormatterTestCases[] = {
"module mdi;"
"import \"DPI-C\" function int add(\n) ;"
"import \"DPI-C\"\t\tfunction int\nsleep( input int secs );"
"import \"DPI-C\"\t\tfunction int\nwake( input int secs, output bit "
"[2:0] z);"
"endmodule",
"module mdi;\n"
" import \"DPI-C\" function int add();\n"
" import \"DPI-C\" function int sleep(\n" // doesn't fit in 40-col
" input int secs);\n"
" import \"DPI-C\" function int sleep(\n"
" input int secs\n"
" );\n"
" import \"DPI-C\" function int wake(\n"
" input int secs,\n"
" output bit [2:0] z\n"
" );\n"
"endmodule\n",
},
{
// DPI export declarations in modules
"module m;"
"export \"DPI-C\" function get;"
"export \"DPI-C\" function mhpmcounter_get; endmodule",
"export \"DPI-C\" function mhpmcounter_get;\n"
"export \"DPI-C\"\t\tfunction int\nwake( input int secs, output bit "
"[2:0] z);"
"endmodule",
"module m;\n"
" export \"DPI-C\" function get;\n"
" export \"DPI-C\"\n"
" function mhpmcounter_get;\n" // doesn't fit in 40-col
" export \"DPI-C\" function int wake(\n"
" input int secs,\n"
" output bit [2:0] z\n"
" );\n"
"endmodule\n",
},
{"import \"DPI-C\" context function void func(input bit impl_i,"
"input bit op_i,"
"input bit [5:0] mode_i,"
"input bit [3:0][31:0] iv_i,"
"input bit [2:0] key_len_i,"
"input bit [7:0][31:0] key_i,"
"input bit [7:0] data_i[],"
"output bit [7:0] data_o[]);",
"import \"DPI-C\" context\n"
" function void func(\n"
" input bit impl_i,\n"
" input bit op_i,\n"
" input bit [5:0] mode_i,\n"
" input bit [3:0][31:0] iv_i,\n"
" input bit [2:0] key_len_i,\n"
" input bit [7:0][31:0] key_i,\n"
" input bit [7:0] data_i [],\n"
" output bit [7:0] data_o []\n"
");\n"},
{// module with system task call
"module m; initial begin #10 $display(\"foo\"); $display(\"bar\");"
"end endmodule",
Expand Down
34 changes: 29 additions & 5 deletions verilog/formatting/tree_unwrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -953,11 +953,21 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions(
case NodeEnum::kTaskHeader:
case NodeEnum::kFunctionHeader:
case NodeEnum::kTaskPrototype:
case NodeEnum::kFunctionPrototype:
case NodeEnum::kFunctionPrototype: {
if (Context().IsInside(NodeEnum::kDPIExportItem) ||
Context().IsInside(NodeEnum::kDPIImportItem)) {
VisitIndentedSection(node, 0,
PartitionPolicyEnum::kFitOnLineElseExpand);
} else {
VisitIndentedSection(node, 0,
PartitionPolicyEnum::kAppendFittingSubPartitions);
}
break;
}

case NodeEnum::kDPIExportItem:
case NodeEnum::kDPIImportItem: {
VisitIndentedSection(node, 0,
PartitionPolicyEnum::kAppendFittingSubPartitions);
VisitIndentedSection(node, 0, PartitionPolicyEnum::kAlwaysExpand);
break;
}

Expand Down Expand Up @@ -1011,8 +1021,7 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions(
case NodeEnum::kModportSimplePortsDeclaration:
case NodeEnum::kModportTFPortsDeclaration:
case NodeEnum::kGateInstanceRegisterVariableList:
case NodeEnum::kVariableDeclarationAssignmentList:
case NodeEnum::kPortList: {
case NodeEnum::kVariableDeclarationAssignmentList: {
// Do not further indent preprocessor clauses.
const int indent = suppress_indentation ? 0 : style_.wrap_spaces;
VisitIndentedSection(node, indent,
Expand All @@ -1033,6 +1042,20 @@ void TreeUnwrapper::SetIndentationsAndCreatePartitions(
}
}

case NodeEnum::kPortList: {
if (Context().IsInside(NodeEnum::kDPIExportItem) ||
Context().IsInside(NodeEnum::kDPIImportItem)) {
const int indent = suppress_indentation ? 0 : style_.indentation_spaces;
VisitIndentedSection(node, indent,
PartitionPolicyEnum::kTabularAlignment);
} else {
const int indent = suppress_indentation ? 0 : style_.wrap_spaces;
VisitIndentedSection(node, indent,
PartitionPolicyEnum::kFitOnLineElseExpand);
}
break;
}

case NodeEnum::kBlockItemStatementList:
case NodeEnum::kStatementList:
case NodeEnum::kFunctionItemList:
Expand Down Expand Up @@ -1513,6 +1536,7 @@ void TreeUnwrapper::ReshapeTokenPartitions(
// Push the "import..." down.
{
verible::MergeLeafIntoNextLeaf(&partition.Children().front());
AttachTrailingSemicolonToPreviousPartition(&partition);
break;
}
case NodeEnum::kBindDirective: {
Expand Down
2 changes: 1 addition & 1 deletion verilog/formatting/tree_unwrapper_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2402,7 +2402,7 @@ const TreeUnwrapperTestData kUnwrapModuleTestCases[] = {
N(1, //
L(1, {"import", "\"DPI-C\"", "function", "int",
"sleep", "("}),
L(3, {"input", "int", "secs", ")", ";"}))),
L(2, {"input", "int", "secs"}), L(1, {")", ";"}))),
L(0, {"endmodule"})),
},

Expand Down
4 changes: 2 additions & 2 deletions verilog/parser/verilog.y
Original file line number Diff line number Diff line change
Expand Up @@ -2737,7 +2737,7 @@ dpi_export_item
: TK_export dpi_spec_string GenericIdentifier '=' modport_tf_port ';'
{ $$ = MakeTaggedNode(N::kDPIExportItem, $1, $2, $3, $4, $5, $6); }
| TK_export dpi_spec_string modport_tf_port ';'
{ $$ = MakeTaggedNode(N::kDPIExportItem, $1, $2, nullptr, nullptr, ExtendLastSublist($3, $4)); }
{ $$ = MakeTaggedNode(N::kDPIExportItem, $1, $2, nullptr, nullptr, $3, $4); }
;
import_export
: TK_export
Expand All @@ -2754,7 +2754,7 @@ dpi_import_item
GenericIdentifier '=' method_prototype ';'
{ $$ = MakeDPIImport($1, $2, $3, $4, $5, ExtendLastSublist($6, $7)); }
| TK_import dpi_spec_string dpi_import_property_opt method_prototype ';'
{ $$ = MakeDPIImport($1, $2, $3, nullptr, nullptr, ExtendLastSublist($4, $5)); }
{ $$ = MakeDPIImport($1, $2, $3, nullptr, nullptr, $4, $5); }
;

modport_ports_declaration_trailing_comma
Expand Down

0 comments on commit e2d7e72

Please sign in to comment.