[clang] 74cc438 - [clang-format] Add option for having one port per line in Verilog
via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 4 07:52:05 PDT 2023
Author: sstwcw
Date: 2023-04-04T14:51:22Z
New Revision: 74cc4389f37d753bf07f1b4a4a4c5e09433d9231
URL: https://github.com/llvm/llvm-project/commit/74cc4389f37d753bf07f1b4a4a4c5e09433d9231
DIFF: https://github.com/llvm/llvm-project/commit/74cc4389f37d753bf07f1b4a4a4c5e09433d9231.diff
LOG: [clang-format] Add option for having one port per line in Verilog
We added the option `VerilogBreakBetweenInstancePorts` to put ports on
separate lines in module instantiations. We made it default to true
because style guides mostly recommend it that way for example:
https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#module-instantiation
Reviewed By: HazardyKnusperkeks
Differential Revision: https://reviews.llvm.org/D147327
Added:
Modified:
clang/docs/ClangFormatStyleOptions.rst
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/unittests/Format/ConfigParseTest.cpp
clang/unittests/Format/FormatTestVerilog.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp
Removed:
################################################################################
diff --git a/clang/docs/ClangFormatStyleOptions.rst b/clang/docs/ClangFormatStyleOptions.rst
index 4078019f82bad..527bdff5f5b56 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -5302,6 +5302,22 @@ the configuration (without a prefix: ``Auto``).
+.. _VerilogBreakBetweenInstancePorts:
+
+**VerilogBreakBetweenInstancePorts** (``Boolean``) :versionbadge:`clang-format 17` :ref:`¶ <VerilogBreakBetweenInstancePorts>`
+ For Verilog, put each port on its own line in module instantiations.
+
+ .. code-block:: c++
+
+ true:
+ ffnand ff1(.q(),
+ .qbar(out1),
+ .clear(in1),
+ .preset(in2));
+
+ false:
+ ffnand ff1(.q(), .qbar(out1), .clear(in1), .preset(in2));
+
.. _WhitespaceSensitiveMacros:
**WhitespaceSensitiveMacros** (``List of Strings``) :versionbadge:`clang-format 11` :ref:`¶ <WhitespaceSensitiveMacros>`
diff --git a/clang/include/clang/Format/Format.h b/clang/include/clang/Format/Format.h
index 85daa50b5bac8..755621ec53d63 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -4219,6 +4219,20 @@ struct FormatStyle {
/// \version 3.7
UseTabStyle UseTab;
+ /// For Verilog, put each port on its own line in module instantiations.
+ /// \code
+ /// true:
+ /// ffnand ff1(.q(),
+ /// .qbar(out1),
+ /// .clear(in1),
+ /// .preset(in2));
+ ///
+ /// false:
+ /// ffnand ff1(.q(), .qbar(out1), .clear(in1), .preset(in2));
+ /// \endcode
+ /// \version 17
+ bool VerilogBreakBetweenInstancePorts;
+
/// A vector of macros which are whitespace-sensitive and should not
/// be touched.
///
@@ -4388,6 +4402,8 @@ struct FormatStyle {
StatementAttributeLikeMacros == R.StatementAttributeLikeMacros &&
StatementMacros == R.StatementMacros && TabWidth == R.TabWidth &&
TypenameMacros == R.TypenameMacros && UseTab == R.UseTab &&
+ VerilogBreakBetweenInstancePorts ==
+ R.VerilogBreakBetweenInstancePorts &&
WhitespaceSensitiveMacros == R.WhitespaceSensitiveMacros;
}
diff --git a/clang/lib/Format/Format.cpp b/clang/lib/Format/Format.cpp
index 97c4867fcb449..248d3684d01f2 100644
--- a/clang/lib/Format/Format.cpp
+++ b/clang/lib/Format/Format.cpp
@@ -1038,6 +1038,8 @@ template <> struct MappingTraits<FormatStyle> {
IO.mapOptional("TabWidth", Style.TabWidth);
IO.mapOptional("TypenameMacros", Style.TypenameMacros);
IO.mapOptional("UseTab", Style.UseTab);
+ IO.mapOptional("VerilogBreakBetweenInstancePorts",
+ Style.VerilogBreakBetweenInstancePorts);
IO.mapOptional("WhitespaceSensitiveMacros",
Style.WhitespaceSensitiveMacros);
IO.mapOptional("Macros", Style.Macros);
@@ -1462,6 +1464,7 @@ FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
LLVMStyle.StatementMacros.push_back("QT_REQUIRE_VERSION");
LLVMStyle.TabWidth = 8;
LLVMStyle.UseTab = FormatStyle::UT_Never;
+ LLVMStyle.VerilogBreakBetweenInstancePorts = true;
LLVMStyle.WhitespaceSensitiveMacros.push_back("BOOST_PP_STRINGIZE");
LLVMStyle.WhitespaceSensitiveMacros.push_back("CF_SWIFT_NAME");
LLVMStyle.WhitespaceSensitiveMacros.push_back("NS_SWIFT_NAME");
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 59d4a871f7eca..dc136428082e0 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -152,6 +152,9 @@ namespace format {
* In 'logic [1:0] x[1:0]', only the first '['. This way we can have space \
* before the first bracket but not the second. */ \
TYPE(VerilogDimensionedTypeName) \
+ /* list of port connections or parameters in a module instantiation */ \
+ TYPE(VerilogInstancePortComma) \
+ TYPE(VerilogInstancePortLParen) \
/* for the base in a number literal, not including the quote */ \
TYPE(VerilogNumberBase) \
/* like `(strong1, pull0)` */ \
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 5171952dba43c..e7de8fd9a2174 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -311,6 +311,9 @@ class AnnotatingParser {
bool OperatorCalledAsMemberFunction =
Prev->Previous && Prev->Previous->isOneOf(tok::period, tok::arrow);
Contexts.back().IsExpression = OperatorCalledAsMemberFunction;
+ } else if (OpeningParen.is(TT_VerilogInstancePortLParen)) {
+ Contexts.back().IsExpression = true;
+ Contexts.back().ContextType = Context::VerilogInstancePortList;
} else if (Style.isJavaScript() &&
(Line.startsWith(Keywords.kw_type, tok::identifier) ||
Line.startsWith(tok::kw_export, Keywords.kw_type,
@@ -1143,6 +1146,52 @@ class AnnotatingParser {
Tok->setType(TT_OverloadedOperatorLParen);
}
+ if (Style.isVerilog()) {
+ // Identify the parameter list and port list in a module instantiation.
+ // This is still needed when we already have
+ // UnwrappedLineParser::parseVerilogHierarchyHeader because that
+ // function is only responsible for the definition, not the
+ // instantiation.
+ auto IsInstancePort = [&]() {
+ const FormatToken *Prev = Tok->getPreviousNonComment();
+ const FormatToken *PrevPrev;
+ // In the following example all 4 left parentheses will be treated as
+ // 'TT_VerilogInstancePortLParen'.
+ //
+ // module_x instance_1(port_1); // Case A.
+ // module_x #(parameter_1) // Case B.
+ // instance_2(port_1), // Case C.
+ // instance_3(port_1); // Case D.
+ if (!Prev || !(PrevPrev = Prev->getPreviousNonComment()))
+ return false;
+ // Case A.
+ if (Keywords.isVerilogIdentifier(*Prev) &&
+ Keywords.isVerilogIdentifier(*PrevPrev)) {
+ return true;
+ }
+ // Case B.
+ if (Prev->is(Keywords.kw_verilogHash) &&
+ Keywords.isVerilogIdentifier(*PrevPrev)) {
+ return true;
+ }
+ // Case C.
+ if (Keywords.isVerilogIdentifier(*Prev) && PrevPrev->is(tok::r_paren))
+ return true;
+ // Case D.
+ if (Keywords.isVerilogIdentifier(*Prev) && PrevPrev->is(tok::comma)) {
+ const FormatToken *PrevParen = PrevPrev->getPreviousNonComment();
+ if (PrevParen->is(tok::r_paren) && PrevParen->MatchingParen &&
+ PrevParen->MatchingParen->is(TT_VerilogInstancePortLParen)) {
+ return true;
+ }
+ }
+ return false;
+ };
+
+ if (IsInstancePort())
+ Tok->setFinalizedType(TT_VerilogInstancePortLParen);
+ }
+
if (!parseParens())
return false;
if (Line.MustBeDeclaration && Contexts.size() == 1 &&
@@ -1286,6 +1335,9 @@ class AnnotatingParser {
case Context::InheritanceList:
Tok->setType(TT_InheritanceComma);
break;
+ case Context::VerilogInstancePortList:
+ Tok->setFinalizedType(TT_VerilogInstancePortComma);
+ break;
default:
if (Style.isVerilog() && Contexts.size() == 1 &&
Line.startsWith(Keywords.kw_assign)) {
@@ -1673,6 +1725,8 @@ class AnnotatingParser {
TemplateArgument,
// C11 _Generic selection.
C11GenericSelection,
+ // Like in the outer parentheses in `ffnand ff1(.q());`.
+ VerilogInstancePortList,
} ContextType = Unknown;
};
@@ -4741,6 +4795,15 @@ bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
// Break between ports of
diff erent types.
if (Left.is(TT_VerilogTypeComma))
return true;
+ // Break between ports in a module instantiation and after the parameter
+ // list.
+ if (Style.VerilogBreakBetweenInstancePorts &&
+ (Left.is(TT_VerilogInstancePortComma) ||
+ (Left.is(tok::r_paren) && Keywords.isVerilogIdentifier(Right) &&
+ Left.MatchingParen &&
+ Left.MatchingParen->is(TT_VerilogInstancePortLParen)))) {
+ return true;
+ }
// Break after labels. In Verilog labels don't have the 'case' keyword, so
// it is hard to identify them in UnwrappedLineParser.
if (!Keywords.isVerilogBegin(Right) && Keywords.isVerilogEndOfLabel(Left))
diff --git a/clang/unittests/Format/ConfigParseTest.cpp b/clang/unittests/Format/ConfigParseTest.cpp
index 6043bce250738..3d5e41776fca0 100644
--- a/clang/unittests/Format/ConfigParseTest.cpp
+++ b/clang/unittests/Format/ConfigParseTest.cpp
@@ -192,6 +192,7 @@ TEST(ConfigParseTest, ParsesConfigurationBools) {
CHECK_PARSE_BOOL(SpaceBeforeJsonColon);
CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets);
+ CHECK_PARSE_BOOL(VerilogBreakBetweenInstancePorts);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterCaseLabel);
CHECK_PARSE_NESTED_BOOL(BraceWrapping, AfterClass);
diff --git a/clang/unittests/Format/FormatTestVerilog.cpp b/clang/unittests/Format/FormatTestVerilog.cpp
index 9d3ccf9f1da04..5e1d8649480b4 100644
--- a/clang/unittests/Format/FormatTestVerilog.cpp
+++ b/clang/unittests/Format/FormatTestVerilog.cpp
@@ -673,6 +673,77 @@ TEST_F(FormatTestVerilog, If) {
" x = x;");
}
+TEST_F(FormatTestVerilog, Instantiation) {
+ // Without ports.
+ verifyFormat("ffnand ff1;");
+ // With named ports.
+ verifyFormat("ffnand ff1(.qbar(out1),\n"
+ " .clear(in1),\n"
+ " .preset(in2));");
+ // With wildcard.
+ verifyFormat("ffnand ff1(.qbar(out1),\n"
+ " .clear(in1),\n"
+ " .preset(in2),\n"
+ " .*);");
+ verifyFormat("ffnand ff1(.*,\n"
+ " .qbar(out1),\n"
+ " .clear(in1),\n"
+ " .preset(in2));");
+ // With unconnected ports.
+ verifyFormat("ffnand ff1(.q(),\n"
+ " .qbar(out1),\n"
+ " .clear(in1),\n"
+ " .preset(in2));");
+ verifyFormat("ffnand ff1(.q(),\n"
+ " .qbar(),\n"
+ " .clear(),\n"
+ " .preset());");
+ verifyFormat("ffnand ff1(,\n"
+ " .qbar(out1),\n"
+ " .clear(in1),\n"
+ " .preset(in2));");
+ // With positional ports.
+ verifyFormat("ffnand ff1(out1,\n"
+ " in1,\n"
+ " in2);");
+ verifyFormat("ffnand ff1(,\n"
+ " out1,\n"
+ " in1,\n"
+ " in2);");
+ // Multiple instantiations.
+ verifyFormat("ffnand ff1(.q(),\n"
+ " .qbar(out1),\n"
+ " .clear(in1),\n"
+ " .preset(in2)),\n"
+ " ff1(.q(),\n"
+ " .qbar(out1),\n"
+ " .clear(in1),\n"
+ " .preset(in2));");
+ verifyFormat("ffnand //\n"
+ " ff1(.q(),\n"
+ " .qbar(out1),\n"
+ " .clear(in1),\n"
+ " .preset(in2)),\n"
+ " ff1(.q(),\n"
+ " .qbar(out1),\n"
+ " .clear(in1),\n"
+ " .preset(in2));");
+ // With breaking between instance ports disabled.
+ auto Style = getDefaultStyle();
+ Style.VerilogBreakBetweenInstancePorts = false;
+ verifyFormat("ffnand ff1;", Style);
+ verifyFormat("ffnand ff1(.qbar(out1), .clear(in1), .preset(in2), .*);",
+ Style);
+ verifyFormat("ffnand ff1(out1, in1, in2);", Style);
+ verifyFormat("ffnand ff1(.q(), .qbar(out1), .clear(in1), .preset(in2)),\n"
+ " ff1(.q(), .qbar(out1), .clear(in1), .preset(in2));",
+ Style);
+ verifyFormat("ffnand //\n"
+ " ff1(.q(), .qbar(out1), .clear(in1), .preset(in2)),\n"
+ " ff1(.q(), .qbar(out1), .clear(in1), .preset(in2));",
+ Style);
+}
+
TEST_F(FormatTestVerilog, Operators) {
// Test that unary operators are not followed by space.
verifyFormat("x = +x;");
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index dc2fd4d10c76c..0fc6442773936 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1645,6 +1645,18 @@ TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
EXPECT_TOKEN_PRECEDENCE(Tokens[1], prec::Assignment);
EXPECT_TOKEN(Tokens[3], tok::lessequal, TT_BinaryOperator);
EXPECT_TOKEN_PRECEDENCE(Tokens[3], prec::Relational);
+
+ // Port lists in module instantiation.
+ Tokens = Annotate("module_x instance_1(port_1), instance_2(port_2);");
+ ASSERT_EQ(Tokens.size(), 12u);
+ EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_VerilogInstancePortLParen);
+ EXPECT_TOKEN(Tokens[7], tok::l_paren, TT_VerilogInstancePortLParen);
+ Tokens = Annotate("module_x #(parameter) instance_1(port_1), "
+ "instance_2(port_2);");
+ ASSERT_EQ(Tokens.size(), 16u);
+ EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_VerilogInstancePortLParen);
+ EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_VerilogInstancePortLParen);
+ EXPECT_TOKEN(Tokens[11], tok::l_paren, TT_VerilogInstancePortLParen);
}
TEST_F(TokenAnnotatorTest, UnderstandConstructors) {
More information about the cfe-commits
mailing list