Merge pull request #2552 from korrenhannes/korrenhannes/codex-fix-verible-2430-doc-link
docs: fix lint rule development guide link
diff --git a/verible/common/formatting/align.h b/verible/common/formatting/align.h
index d042ae0..f290ac6 100644
--- a/verible/common/formatting/align.h
+++ b/verible/common/formatting/align.h
@@ -407,22 +407,33 @@
// Skip tree tokens. Non-tree tokens located between tree tokens (e.g. block
// comments) are also skipped.
while (*(ftoken_it->token) != last_tree_token) ++ftoken_it;
- // Use next token as begining of trailing non-tree tokens
+ // Use next token as beginning of trailing non-tree tokens
trailing_tokens.set_begin(ftoken_it + 1);
- // Breaking following condition leads to e.g. concatenation of EOL comment
- // and code in a single line. To fix situation that lead to this, flatten
- // token partitions that contain EOL comment subpartition just before a
- // subpartition that starts with the same token as Origin(). Example of a
- // partition that needs flattening:
+ // When leading non-tree tokens (typically a comment block between list
+ // items) coexist with the first tree token being forced to start on a
+ // new line, the leading tokens belong on the preceding line and the
+ // tree content on a separate line. Discard the leading tokens from
+ // alignment consideration instead of trying to align them.
+ // (This can also arise from EOL comments in a partition that needs
+ // flattening; see the example below.)
//
+ // Example of a partition tree that needs flattening (from the tree
+ // unwrapper, not this code):
// { (>>[...], (origin: "input bit second"))
// { (>>[// comment] }
// { (>>[input bit second], (origin: "input")) }
// }
- CHECK(leading_tokens.empty() || first_tree_token_it == ftokens.end() ||
- first_tree_token_it->before.break_decision !=
- SpacingOptions::kMustWrap);
+ if (!leading_tokens.empty() && first_tree_token_it != ftokens.end() &&
+ first_tree_token_it->before.break_decision ==
+ SpacingOptions::kMustWrap) {
+ // The partition has comment tokens that were absorbed into the
+ // following item's partition (a tree-unwrapper flattening gap).
+ // Return tree-scanned columns without non-tree additions,
+ // effectively skipping alignment for this row rather than
+ // risking corruption of the layout.
+ return column_entries;
+ }
} else {
// All tokens are passed as leading
leading_tokens.set_end(ftokens.end());
diff --git a/verible/common/strings/patch.cc b/verible/common/strings/patch.cc
index 966307b..aced6a0 100644
--- a/verible/common/strings/patch.cc
+++ b/verible/common/strings/patch.cc
@@ -229,7 +229,8 @@
int line_number = header_.old_range.start; // 1-indexed
for (const MarkedLine &line : lines_) {
if (line.IsAdded()) continue; // ignore added lines
- if (line_number > static_cast<int>(original_lines.size())) {
+ if (line_number < 1 ||
+ line_number > static_cast<int>(original_lines.size())) {
return absl::OutOfRangeError(absl::StrCat(
"Patch hunk references line ", line_number, " in a file with only ",
original_lines.size(), " lines"));
diff --git a/verible/common/strings/patch_test.cc b/verible/common/strings/patch_test.cc
index 17a4930..a3c9ae3 100644
--- a/verible/common/strings/patch_test.cc
+++ b/verible/common/strings/patch_test.cc
@@ -755,6 +755,34 @@
}
}
+TEST(HunkVerifyAgainstOriginalLinesTest, NonPositiveStartLine) {
+ // A hunk header whose old-file start is 0 (line numbers are 1-indexed) must
+ // be rejected rather than indexing original_lines at a non-positive offset.
+ const std::vector<std::string_view> kHunkText = {
+ {
+ "@@ -0,2 +1,2 @@", //
+ " line1", // retained line reached with line_number == 0
+ "-line2", //
+ "+line pi", //
+ },
+ };
+ const std::vector<std::string_view> kOriginal = {
+ "line1",
+ "line2",
+ };
+ Hunk hunk;
+ {
+ const LineRange range(kHunkText.begin(), kHunkText.end());
+ const auto status = hunk.Parse(range);
+ ASSERT_TRUE(status.ok()) << status.message();
+ }
+ {
+ const auto status = hunk.VerifyAgainstOriginalLines(kOriginal);
+ EXPECT_EQ(status.code(), absl::StatusCode::kOutOfRange);
+ EXPECT_TRUE(absl::StrContains(status.message(), "references line 0"));
+ }
+}
+
TEST(HunkVerifyAgainstOriginalLinesTest, InconsistentRetainedLine) {
const std::vector<std::string_view> kHunkText = {
{
diff --git a/verible/verilog/CST/BUILD b/verible/verilog/CST/BUILD
index 39207e0..03aa2bf 100644
--- a/verible/verilog/CST/BUILD
+++ b/verible/verilog/CST/BUILD
@@ -749,6 +749,8 @@
"//verible/common/text:symbol",
"//verible/common/text:tree-utils",
"//verible/common/text:visitors",
+ "//verible/common/util:casts",
+ "//verible/verilog/parser:verilog-token-enum",
],
)
diff --git a/verible/verilog/CST/statement.cc b/verible/verilog/CST/statement.cc
index 0dce48c..b3923bd 100644
--- a/verible/verilog/CST/statement.cc
+++ b/verible/verilog/CST/statement.cc
@@ -21,11 +21,13 @@
#include "verible/common/text/concrete-syntax-tree.h"
#include "verible/common/text/symbol.h"
#include "verible/common/text/tree-utils.h"
+#include "verible/common/util/casts.h"
#include "verible/verilog/CST/declaration.h"
#include "verible/verilog/CST/identifier.h"
#include "verible/verilog/CST/type.h"
#include "verible/verilog/CST/verilog-matchers.h" // IWYU pragma: keep
#include "verible/verilog/CST/verilog-nonterminals.h"
+#include "verible/verilog/parser/verilog-token-enum.h"
namespace verilog {
@@ -447,6 +449,21 @@
}
// Returns the data type node from for loop initialization.
+const verible::SyntaxTreeLeaf *GetGenvarKeywordFromForInitialization(
+ const verible::Symbol &for_initialization) {
+ const verible::Symbol *child0 =
+ GetSubtreeAsSymbol(for_initialization, NodeEnum::kForInitialization, 0);
+ if (child0 == nullptr) return nullptr;
+ if (child0->Kind() == verible::SymbolKind::kLeaf) {
+ const auto *leaf =
+ verible::down_cast<const verible::SyntaxTreeLeaf *>(child0);
+ if (leaf->get().token_enum() == TK_genvar) {
+ return leaf;
+ }
+ }
+ return nullptr;
+}
+
const verible::SyntaxTreeNode *GetDataTypeFromForInitialization(
const verible::Symbol &for_initialization) {
const auto *data_type = verible::GetSubtreeAsSymbol(
diff --git a/verible/verilog/CST/statement.h b/verible/verilog/CST/statement.h
index 2b8ba39..27fe670 100644
--- a/verible/verilog/CST/statement.h
+++ b/verible/verilog/CST/statement.h
@@ -179,8 +179,13 @@
const verible::Symbol &conditional);
// Returns the data type node from for loop initialization.
+// Returns the genvar keyword leaf from a for initialization, or nullptr if not
+// present.
+const verible::SyntaxTreeLeaf *GetGenvarKeywordFromForInitialization(
+ const verible::Symbol &for_initialization);
+
const verible::SyntaxTreeNode *GetDataTypeFromForInitialization(
- const verible::Symbol &);
+ const verible::Symbol &for_initialization);
// Returns the variable name leaf from for loop initialization.
const verible::SyntaxTreeLeaf *GetVariableNameFromForInitialization(
diff --git a/verible/verilog/CST/verilog-matchers.h b/verible/verilog/CST/verilog-matchers.h
index bc41a60..6663e4a 100644
--- a/verible/verilog/CST/verilog-matchers.h
+++ b/verible/verilog/CST/verilog-matchers.h
@@ -437,6 +437,8 @@
// ...
inline constexpr auto HasUniqueQualifier =
verible::matcher::MakePathMatcher(L(TK_unique));
+inline constexpr auto HasUnique0Qualifier =
+ verible::matcher::MakePathMatcher(L(TK_unique0));
// Clean up macros
#undef N
#undef L
diff --git a/verible/verilog/CST/verilog-matchers_test.cc b/verible/verilog/CST/verilog-matchers_test.cc
index c38edff..bb0a4c9 100644
--- a/verible/verilog/CST/verilog-matchers_test.cc
+++ b/verible/verilog/CST/verilog-matchers_test.cc
@@ -822,10 +822,69 @@
endfunction
)",
1},
+ {HasUniqueQualifier(),
+ R"(
+ function automatic int foo (input in);
+ unique0 case (in)
+ 1: return 0;
+ endcase
+ endfunction
+ )",
+ 0},
};
for (const auto &test : tests) {
verible::matcher::RunRawMatcherTestCase<VerilogAnalyzer>(test);
}
}
+
+// Tests for HasUnique0Qualifier matching.
+TEST(VerilogMatchers, HasUnique0QualifierTests) {
+ const RawMatcherTestCase tests[] = {
+ {HasUnique0Qualifier(), "", 0},
+ {HasUnique0Qualifier(),
+ R"(
+ function automatic int foo (input in);
+ case (in)
+ default: return 0;
+ endcase
+ endfunction
+ )",
+ 0},
+ {HasUnique0Qualifier(),
+ R"(
+ function automatic int foo (input in);
+ unique0 case (in)
+ 1: return 0;
+ endcase
+ endfunction
+ )",
+ 1},
+ {HasUnique0Qualifier(),
+ R"(
+ function automatic int foo (input in);
+ unique case (in)
+ 1: return 0;
+ endcase
+ endfunction
+ )",
+ 0},
+ {HasUnique0Qualifier(),
+ R"(
+ function automatic int foo (input in);
+ unique0 if (in) begin
+ return 0;
+ end
+ else if(!in) begin
+ return 1;
+ end
+ endfunction
+ )",
+ 1},
+ };
+ for (const auto &test : tests) {
+ verible::matcher::RunRawMatcherTestCase<VerilogAnalyzer>(test);
+ }
+}
+
} // namespace
} // namespace verilog
diff --git a/verible/verilog/analysis/checkers/case-missing-default-rule.cc b/verible/verilog/analysis/checkers/case-missing-default-rule.cc
index c401cd6..8f8ec39 100644
--- a/verible/verilog/analysis/checkers/case-missing-default-rule.cc
+++ b/verible/verilog/analysis/checkers/case-missing-default-rule.cc
@@ -42,7 +42,7 @@
static constexpr std::string_view kMessage =
"Explicitly define a default case for every case statement or add `unique` "
- "qualifier to the case statement.";
+ "or `unique0` qualifier to the case statement.";
const LintRuleDescriptor &CaseMissingDefaultRule::GetDescriptor() {
static const LintRuleDescriptor d{
@@ -50,7 +50,7 @@
.topic = "case-statements",
.desc =
"Checks that a default case-item is always defined unless the case "
- "statement has the `unique` qualifier.",
+ "statement has the `unique` or `unique0` qualifier.",
};
return d;
}
@@ -67,12 +67,16 @@
static const Matcher uniqueCaseMatcher(
NodekCaseStatement(HasUniqueQualifier()));
+ static const Matcher unique0CaseMatcher(
+ NodekCaseStatement(HasUnique0Qualifier()));
+
static const Matcher caseMatcherWithDefaultCase(
NodekCaseStatement(HasDefaultCase()));
// If the case statement doesn't have the "unique" qualifier and
// it is missing the "default" case, insert the violation
if (!uniqueCaseMatcher.Matches(symbol, &manager) &&
+ !unique0CaseMatcher.Matches(symbol, &manager) &&
!caseMatcherWithDefaultCase.Matches(symbol, &manager)) {
violations_.insert(LintViolation(symbol, kMessage, context));
}
diff --git a/verible/verilog/analysis/checkers/case-missing-default-rule_test.cc b/verible/verilog/analysis/checkers/case-missing-default-rule_test.cc
index 2b465df..84b8e77 100644
--- a/verible/verilog/analysis/checkers/case-missing-default-rule_test.cc
+++ b/verible/verilog/analysis/checkers/case-missing-default-rule_test.cc
@@ -52,6 +52,22 @@
endfunction
)"},
{R"(
+ function automatic int foo (input [2:0] in);
+ unique case (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
+ function automatic int foo (input [2:0] in);
+ unique0 case (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
function automatic int foo (input in);
)",
{TK_case, "case"},
@@ -70,6 +86,22 @@
endfunction
)"},
{R"(
+ function automatic int foo (input [2:0] in);
+ unique casex (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
+ function automatic int foo (input [2:0] in);
+ unique0 casex (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
function automatic int foo (input in);
)",
{TK_casex, "casex"},
@@ -88,6 +120,22 @@
endfunction
)"},
{R"(
+ function automatic int foo (input [2:0] in);
+ unique casez (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
+ function automatic int foo (input [2:0] in);
+ unique0 casez (in)
+ 3'b001: return 1;
+ endcase
+ return 0;
+ endfunction
+ )"},
+ {R"(
function automatic int foo (input in);
)",
{TK_casez, "casez"},
@@ -96,6 +144,20 @@
endcase
endfunction
)"},
+ // A qualified outer case does not exempt an unqualified inner case.
+ {R"(
+ function automatic int foo (input in);
+ unique0 case (in)
+ 1: begin;
+ )",
+ {TK_case, "case"},
+ R"( (in)
+ 1: return 1;
+ endcase
+ end
+ endcase
+ endfunction
+ )"},
// randcase should not be flagged
{R"(
diff --git a/verible/verilog/analysis/symbol-table.cc b/verible/verilog/analysis/symbol-table.cc
index 9847f46..7d51108 100644
--- a/verible/verilog/analysis/symbol-table.cc
+++ b/verible/verilog/analysis/symbol-table.cc
@@ -89,6 +89,9 @@
{"class", SymbolMetaType::kClass},
{"module", SymbolMetaType::kModule},
{"package", SymbolMetaType::kPackage},
+ {"generate", SymbolMetaType::kGenerate},
+ {"procedural block", SymbolMetaType::kProceduralBlock},
+ {"loop scope", SymbolMetaType::kLoopScope},
{"parameter", SymbolMetaType::kParameter},
{"typedef", SymbolMetaType::kTypeAlias},
{"data/net/var/instance", SymbolMetaType::kDataNetVariableInstance},
@@ -98,6 +101,8 @@
{"enum", SymbolMetaType::kEnumType},
{"<enum constant>", SymbolMetaType::kEnumConstant},
{"interface", SymbolMetaType::kInterface},
+ {"genvar", SymbolMetaType::kGenvar},
+ {"generate loop", SymbolMetaType::kGenerateLoop},
{"<unspecified>", SymbolMetaType::kUnspecified},
{"<callable>", SymbolMetaType::kCallable},
});
@@ -210,6 +215,18 @@
case NodeEnum::kGenerateElseClause:
DeclareGenerateElse(node);
break;
+
+ // separate cases for handling different parts of generate construct
+ case NodeEnum::kLoopGenerateConstruct:
+ DeclareGenerateLoop(node);
+ break;
+ case NodeEnum::kGenerateBlock:
+ DeclareGenerateBlock(node);
+ break;
+ case NodeEnum::kGenvarDeclaration:
+ DeclareGenvarDeclaration(node);
+ break;
+
case NodeEnum::kPackageDeclaration:
DeclarePackage(node);
break;
@@ -306,6 +323,18 @@
// TODO(#1241) Not handled right now.
// TODO(#1255) Not handled right now.
break;
+ // separate case for blocks
+ case NodeEnum::kSeqBlock:
+ DeclareSequentialBlock(node);
+ break;
+ // separate case for for-loops
+ case NodeEnum::kForLoopStatement:
+ DeclareForLoop(node);
+ break;
+ // separate case for for-loop initialization
+ case NodeEnum::kForInitialization:
+ DeclareForInitVariable(node);
+ break;
default:
Descend(node);
break;
@@ -1242,28 +1271,17 @@
SymbolMetaType::kModule);
}
- std::string_view GetScopeNameFromGenerateBody(const SyntaxTreeNode &body) {
- if (body.MatchesTag(NodeEnum::kGenerateBlock)) {
- const SyntaxTreeNode *gen_block = GetGenerateBlockBegin(body);
- const TokenInfo *label =
- gen_block ? GetBeginLabelTokenInfo(*gen_block) : nullptr;
- if (label != nullptr) {
- // TODO: Check for a matching end-label here, and if its name matches
- // the begin label, then immediately create a resolved reference because
- // it only makes sense for it resolve to this begin.
- // Otherwise, do nothing with the end label.
- return label->text();
- }
- }
- return current_scope_->Value().CreateAnonymousScope("generate");
- }
-
void DeclareGenerateIf(const SyntaxTreeNode &generate_if) {
const SyntaxTreeNode *body(GetIfClauseGenerateBody(generate_if));
if (body) {
- DeclareScopedElementAndDescend(generate_if,
- GetScopeNameFromGenerateBody(*body),
- SymbolMetaType::kGenerate);
+ if (body->MatchesTag(NodeEnum::kGenerateBlock)) {
+ Descend(generate_if);
+ } else {
+ DeclareScopedElementAndDescend(
+ generate_if,
+ current_scope_->Value().CreateAnonymousScope("generate"),
+ SymbolMetaType::kGenerate);
+ }
}
}
@@ -1276,13 +1294,79 @@
// and let the if-clause inside create a scope directly under the current
// scope.
Descend(*body);
+ } else if (body->MatchesTag(NodeEnum::kGenerateBlock)) {
+ Descend(generate_else);
} else {
- DeclareScopedElementAndDescend(generate_else,
- GetScopeNameFromGenerateBody(*body),
+ DeclareScopedElementAndDescend(
+ generate_else,
+ current_scope_->Value().CreateAnonymousScope("generate"),
+ SymbolMetaType::kGenerate);
+ }
+ }
+
+ void DeclareGenerateLoop(const SyntaxTreeNode &node) {
+ const std::string_view scope_name =
+ current_scope_->Value().CreateAnonymousScope("generate-for");
+ DeclareScopedElementAndDescend(node, scope_name,
+ SymbolMetaType::kGenerateLoop);
+ }
+
+ void DeclareGenerateBlock(const SyntaxTreeNode &node) {
+ const SyntaxTreeNode *gen_block = GetGenerateBlockBegin(node);
+ const TokenInfo *label =
+ gen_block ? GetBeginLabelTokenInfo(*gen_block) : nullptr;
+ if (label != nullptr) {
+ DeclareScopedElementAndDescend(node, label->text(),
+ SymbolMetaType::kGenerate);
+ } else {
+ const std::string_view scope_name =
+ current_scope_->Value().CreateAnonymousScope("generate");
+ DeclareScopedElementAndDescend(node, scope_name,
SymbolMetaType::kGenerate);
}
}
+ void DeclareGenvarDeclaration(const SyntaxTreeNode &node) {
+ const auto *identifier_list =
+ GetSubtreeAsSymbol(node, NodeEnum::kGenvarDeclaration, 1);
+ if (!identifier_list) return;
+
+ const auto *list_node =
+ verible::down_cast<const SyntaxTreeNode *>(identifier_list);
+ for (const auto &child : list_node->children()) {
+ if (child && child->Kind() == verible::SymbolKind::kLeaf) {
+ const SyntaxTreeLeaf &leaf = verible::SymbolCastToLeaf(*child);
+ if (leaf.get().token_enum() == verilog_tokentype::SymbolIdentifier) {
+ EmplaceElementInCurrentScope(leaf, leaf.get().text(),
+ SymbolMetaType::kGenvar);
+ }
+ }
+ }
+ }
+
+ void DeclareSequentialBlock(const SyntaxTreeNode &node) {
+ // Procedural begin...end blocks introduce a new lexical scope.
+ // Extract optional label from the kBegin child for scope naming.
+ const verible::TokenInfo *label = nullptr;
+ if (!node.empty() && node.front() != nullptr) {
+ label = GetBeginLabelTokenInfo(*node.front());
+ }
+ const std::string_view scope_name =
+ label ? label->text()
+ : current_scope_->Value().CreateAnonymousScope("block");
+ DeclareScopedElementAndDescend(node, scope_name,
+ SymbolMetaType::kProceduralBlock);
+ }
+
+ void DeclareForLoop(const SyntaxTreeNode &node) {
+ // for-loop statements introduce a scope that encompasses both the
+ // initialization variables and the loop body.
+ const std::string_view scope_name =
+ current_scope_->Value().CreateAnonymousScope("for");
+ DeclareScopedElementAndDescend(node, scope_name,
+ SymbolMetaType::kLoopScope);
+ }
+
void DeclarePackage(const SyntaxTreeNode &package) {
const auto *token = GetPackageNameToken(package);
if (!token) return;
@@ -1432,6 +1516,43 @@
VLOG(2) << "end of " << __FUNCTION__;
}
+ // Declares a variable from a for-loop initialization (e.g. `int i = 0`).
+ // Handles both explicit `genvar` and standard typed declarations.
+ void DeclareForInitVariable(const SyntaxTreeNode &for_init) {
+ const verible::SyntaxTreeLeaf *genvar_keyword =
+ GetGenvarKeywordFromForInitialization(for_init);
+ if (genvar_keyword) {
+ const verible::SyntaxTreeLeaf *var_name =
+ GetVariableNameFromForInitialization(for_init);
+ if (var_name) {
+ EmplaceElementInCurrentScope(*var_name, var_name->get().text(),
+ SymbolMetaType::kGenvar);
+ }
+ Descend(for_init);
+ return;
+ }
+
+ const verible::SyntaxTreeNode *data_type =
+ GetDataTypeFromForInitialization(for_init);
+ if (data_type == nullptr) {
+ // not a variable declaration. Descend normally for references.
+ Descend(for_init);
+ return;
+ }
+ const verible::SyntaxTreeLeaf *var_name =
+ GetVariableNameFromForInitialization(for_init);
+ if (var_name == nullptr) {
+ Descend(for_init);
+ return;
+ }
+ DeclarationTypeInfo decl_type_info;
+ const ValueSaver<DeclarationTypeInfo *> save_type(&declaration_type_info_,
+ &decl_type_info);
+ Descend(for_init);
+ EmplaceTypedElementInCurrentScope(for_init, var_name->get().text(),
+ SymbolMetaType::kDataNetVariableInstance);
+ }
+
// Declare one (of potentially multiple) instances in a single declaration
// statement.
void DeclareInstance(const SyntaxTreeNode &instance) {
diff --git a/verible/verilog/analysis/symbol-table.h b/verible/verilog/analysis/symbol-table.h
index 6632b81..92fa30d 100644
--- a/verible/verilog/analysis/symbol-table.h
+++ b/verible/verilog/analysis/symbol-table.h
@@ -53,7 +53,9 @@
kRoot,
kClass,
kModule,
- kGenerate, // loop or conditional generate block
+ kGenerate, // loop or conditional generate block
+ kProceduralBlock, // begin...end block
+ kLoopScope, // for loop body and init
kPackage,
kParameter,
kTypeAlias, // typedef
@@ -64,6 +66,8 @@
kEnumType,
kEnumConstant,
kInterface,
+ kGenvar,
+ kGenerateLoop,
// The following enums represent classes/groups of the above types,
// and are used for validating metatypes of symbol references.
diff --git a/verible/verilog/analysis/symbol-table_test.cc b/verible/verilog/analysis/symbol-table_test.cc
index cec3c88..e15ec5f 100644
--- a/verible/verilog/analysis/symbol-table_test.cc
+++ b/verible/verilog/analysis/symbol-table_test.cc
@@ -713,6 +713,320 @@
}
}
+TEST_F(BuildSymbolTableTest, SeqBlockCreatesAnonymousScope) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " initial begin\n"
+ " int x;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ EXPECT_EQ(module_node_info.metatype, SymbolMetaType::kModule);
+
+ // The begin...end block should create a child scope.
+ ASSERT_EQ(module_node.Children().size(), 1);
+ const SymbolTableNode &block(module_node.Children().begin()->second);
+ const SymbolInfo &block_info(block.Value());
+ EXPECT_EQ(block_info.metatype, SymbolMetaType::kProceduralBlock);
+
+ // 'x' should be declared inside the block scope, not module scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_x, block, "x");
+ EXPECT_EQ(var_x_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, SeqBlockLabeledScope) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " initial begin : my_block\n"
+ " int y;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ EXPECT_EQ(module_node_info.metatype, SymbolMetaType::kModule);
+
+ // Labeled block should use the label as scope name.
+ MUST_ASSIGN_LOOKUP_SYMBOL(block_node, module_node, "my_block");
+ EXPECT_EQ(block_node_info.metatype, SymbolMetaType::kProceduralBlock);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_y, block_node, "y");
+ EXPECT_EQ(var_y_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, SeqBlockVariableShadowing) {
+ // Inner begin...end block declares 'x' which shadows the outer module-level
+ // 'x'. Both declarations should succeed (no "already defined" error).
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " int x;\n"
+ " initial begin\n"
+ " int x;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ EXPECT_EQ(module_node_info.metatype, SymbolMetaType::kModule);
+
+ // Outer 'x' in module scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(outer_x, module_node, "x");
+ EXPECT_EQ(outer_x_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ // Block scope exists as a second child of module.
+ EXPECT_EQ(module_node.Children().size(), 2); // x + anonymous block
+ auto iter = module_node.Children().begin();
+ // Find the block child (skip past 'x').
+ while (iter != module_node.Children().end() &&
+ iter->second.Value().metatype != SymbolMetaType::kProceduralBlock) {
+ ++iter;
+ }
+ ASSERT_NE(iter, module_node.Children().end());
+ const SymbolTableNode &block(iter->second);
+
+ // Inner 'x' lives inside the block scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(inner_x, block, "x");
+ EXPECT_EQ(inner_x_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, GenerateForVariableScope) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " for (genvar i = 0; i < 5; i++) begin : gen_blk\n"
+ " int j;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+
+ // Module has one child: the generate for loop scope (anonymous).
+ ASSERT_EQ(module_node.Children().size(), 1);
+ const SymbolTableNode &for_scope(module_node.Children().begin()->second);
+ EXPECT_EQ(for_scope.Value().metatype, SymbolMetaType::kGenerateLoop);
+
+ // 'i' is declared in the for-loop scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_i, for_scope, "i");
+ EXPECT_EQ(var_i_info.metatype, SymbolMetaType::kGenvar);
+
+ // The generate for-loop body (begin...end) creates another nested scope
+ // containing 'j'.
+ MUST_ASSIGN_LOOKUP_SYMBOL(body_block, for_scope, "gen_blk");
+ EXPECT_EQ(body_block_info.metatype, SymbolMetaType::kGenerate);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_j, body_block, "j");
+ EXPECT_EQ(var_j_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, GenerateForVariableScopeAnonymous) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " for (genvar i = 0; i < 5; i++) begin\n"
+ " int j;\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ ASSERT_EQ(module_node.Children().size(), 1);
+ const SymbolTableNode &for_scope(module_node.Children().begin()->second);
+ EXPECT_EQ(for_scope.Value().metatype, SymbolMetaType::kGenerateLoop);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_i, for_scope, "i");
+ EXPECT_EQ(var_i_info.metatype, SymbolMetaType::kGenvar);
+
+ const SymbolTableNode *body_block = nullptr;
+ for (const auto &child : for_scope.Children()) {
+ if (child.second.Value().metatype == SymbolMetaType::kGenerate) {
+ body_block = &child.second;
+ break;
+ }
+ }
+ ASSERT_NE(body_block, nullptr);
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_j, *body_block, "j");
+ EXPECT_EQ(var_j_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+}
+
+TEST_F(BuildSymbolTableTest, StandaloneGenvarDeclaration) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " genvar i, j;\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_i, module_node, "i");
+ EXPECT_EQ(var_i_info.metatype, SymbolMetaType::kGenvar);
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_j, module_node, "j");
+ EXPECT_EQ(var_j_info.metatype, SymbolMetaType::kGenvar);
+}
+
+TEST_F(BuildSymbolTableTest, ForLoopVariableScope) {
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " initial begin\n"
+ " for (int i = 0; i < 5; i++) begin\n"
+ " int j;\n"
+ " end\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+
+ // Module has one child: the initial block scope.
+ ASSERT_EQ(module_node.Children().size(), 1);
+ const SymbolTableNode &init_block(module_node.Children().begin()->second);
+ EXPECT_EQ(init_block.Value().metatype, SymbolMetaType::kProceduralBlock);
+
+ // The initial block has one child: the for-loop scope.
+ ASSERT_EQ(init_block.Children().size(), 1);
+ const SymbolTableNode &for_scope(init_block.Children().begin()->second);
+ EXPECT_EQ(for_scope.Value().metatype, SymbolMetaType::kLoopScope);
+
+ // 'i' is declared in the for-loop scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_i, for_scope, "i");
+ EXPECT_EQ(var_i_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ // The for-loop body (begin...end) creates another nested scope containing
+ // 'j'. Find the block scope child (skip past 'i').
+ const SymbolTableNode *body_block = nullptr;
+ for (const auto &child : for_scope.Children()) {
+ if (child.second.Value().metatype == SymbolMetaType::kProceduralBlock) {
+ body_block = &child.second;
+ break;
+ }
+ }
+ ASSERT_NE(body_block, nullptr);
+ MUST_ASSIGN_LOOKUP_SYMBOL(var_j, *body_block, "j");
+ EXPECT_EQ(var_j_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
+TEST_F(BuildSymbolTableTest, ForLoopVariableShadowing) {
+ // Module-level 'i' and for-loop 'i' should not collide.
+ TestVerilogSourceFile src("foobar.sv",
+ "module m;\n"
+ " int i;\n"
+ " initial begin\n"
+ " for (int i = 0; i < 5; i++) begin\n"
+ " end\n"
+ " end\n"
+ "endmodule\n");
+ const auto status = src.Parse();
+ ASSERT_TRUE(status.ok()) << status.message();
+ SymbolTable symbol_table(nullptr);
+ const SymbolTableNode &root_symbol(symbol_table.Root());
+
+ const auto build_diagnostics = BuildSymbolTable(src, &symbol_table);
+ EXPECT_EMPTY_STATUSES(build_diagnostics);
+
+ MUST_ASSIGN_LOOKUP_SYMBOL(module_node, root_symbol, "m");
+
+ // Module-level 'i' exists.
+ MUST_ASSIGN_LOOKUP_SYMBOL(outer_i, module_node, "i");
+ EXPECT_EQ(outer_i_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ // Find the for-loop scope (nested inside the initial block scope).
+ const SymbolTableNode *for_scope = nullptr;
+ for (const auto &child : module_node.Children()) {
+ if (child.second.Value().metatype == SymbolMetaType::kProceduralBlock) {
+ // This is the initial block. Search inside for the for-loop scope.
+ for (const auto &grandchild : child.second.Children()) {
+ if (grandchild.second.Value().metatype == SymbolMetaType::kLoopScope) {
+ for_scope = &grandchild.second;
+ break;
+ }
+ }
+ break;
+ }
+ }
+ ASSERT_NE(for_scope, nullptr);
+
+ // For-loop 'i' lives in the for-loop scope.
+ MUST_ASSIGN_LOOKUP_SYMBOL(loop_i, *for_scope, "i");
+ EXPECT_EQ(loop_i_info.metatype, SymbolMetaType::kDataNetVariableInstance);
+
+ {
+ std::vector<absl::Status> resolve_diagnostics;
+ symbol_table.Resolve(&resolve_diagnostics);
+ EXPECT_EMPTY_STATUSES(resolve_diagnostics);
+ }
+}
+
TEST_F(BuildSymbolTableTest, ModuleDeclarationWithPorts) {
TestVerilogSourceFile src("foobar.sv",
"module m (\n"
diff --git a/verible/verilog/formatting/align.cc b/verible/verilog/formatting/align.cc
index 1f7f472..6640adb 100644
--- a/verible/verilog/formatting/align.cc
+++ b/verible/verilog/formatting/align.cc
@@ -632,7 +632,9 @@
kDontCare = 0,
kNamedActualParameters,
kNamedActualPorts,
- kParameterDeclaration,
+ kParameterDeclaration, // formal parameter list (#(...))
+ kBodyParameterDeclaration, // parameter/localparam in module/generate/
+ // package/interface body
kPortDeclaration,
kStructUnionMember,
kDataDeclaration, // net/variable declarations
@@ -674,6 +676,13 @@
return AlignClassify(AlignmentGroupAction::kIgnore);
}
const SyntaxTreeNode &node = verible::SymbolCastToNode(*origin);
+ // Align body-level parameter/localparam declarations.
+ if (node.MatchesTag(NodeEnum::kParamDeclaration)) {
+ return AlignClassify(
+ AlignmentGroupAction::kMatch,
+ AlignableSyntaxSubtype::kBodyParameterDeclaration);
+ }
+
// Align net/variable declarations.
if (IsAlignableDeclaration(node)) {
return AlignClassify(AlignmentGroupAction::kMatch,
@@ -837,7 +846,7 @@
}
case NodeEnum::kDimensionSlice:
case NodeEnum::kDimensionAssociativeType: {
- // all of these cases cover packed and unpacked dimensions
+ // All of these cases cover packed and unpacked dimensions
ReserveNewColumn(node, FlushLeft);
break;
}
@@ -951,7 +960,6 @@
CHECK_EQ(node.size(), 5);
auto *column = ABSL_DIE_IF_NULL(ReserveNewColumn(node, FlushLeft));
- SyntaxTreePath np;
ReserveNewColumn(column, *node[0], FlushLeft); // '['
auto *value_subcolumn =
@@ -985,9 +993,8 @@
}
};
-// This class marks up token-subranges in formal parameter declarations for
-// alignment.
-// e.g. "localparam int Width = 5;"
+// This class marks up token-subranges in formal and body-level parameter
+// declarations for alignment. e.g. "localparam int Width = 5;"
class ParameterDeclarationColumnSchemaScanner
: public VerilogColumnSchemaScanner {
public:
@@ -1061,7 +1068,7 @@
break;
}
- // Sometimes the parameter indentifier which is of token SymbolIdentifier
+ // Sometimes the parameter identifier which is of token SymbolIdentifier
// can appear at different paths depending on the parameter type. Make
// them aligned so they fall under the same column.
case verilog_tokentype::SymbolIdentifier: {
@@ -1090,7 +1097,7 @@
break;
}
- // Align packed and unpacked dimenssions
+ // Align packed and unpacked dimensions
case '[': {
if (verilog::analysis::ContextIsInsideDeclarationDimensions(
Context()) &&
@@ -1456,6 +1463,11 @@
ParameterDeclarationColumnSchemaScanner>(non_tree_column_scanner),
function_from_pointer_to_member(
&FormatStyle::formal_parameters_alignment)}},
+ {AlignableSyntaxSubtype::kBodyParameterDeclaration,
+ {UnstyledAlignmentCellScannerGenerator<
+ ParameterDeclarationColumnSchemaScanner>(non_tree_column_scanner),
+ function_from_pointer_to_member(
+ &FormatStyle::parameter_declaration_alignment)}},
{AlignableSyntaxSubtype::kPortDeclaration,
{UnstyledAlignmentCellScannerGenerator<
PortDeclarationColumnSchemaScanner>(non_tree_column_scanner),
@@ -1598,8 +1610,9 @@
static std::vector<AlignablePartitionGroup> AlignModuleItems(
const TokenPartitionRange &full_range, const FormatStyle &vstyle) {
- // Currently, this only handles data/net/variable declarations.
- // TODO(b/161814377): align continuous assignments
+ // Applies to module/interface, generate, and package item lists.
+ // Handles data/net/variable declarations, parameter/localparam
+ // declarations, and continuous assignment statements.
auto group_extractor = [&vstyle](const TokenPartitionRange &range) {
return GetConsecutiveModuleItemGroups(range,
vstyle.alignment_group_boundary);
@@ -1635,6 +1648,10 @@
&IgnoreCommentsAndPreprocessingDirectives, full_range, vstyle);
}
+// Aligns formal parameters in #(...) headers (module/interface/class port
+// parameter lists). Body-level parameter/localparam declarations are
+// handled by AlignModuleItems which dispatches through
+// GetConsecutiveModuleItemGroups.
static std::vector<AlignablePartitionGroup> AlignParameterDeclarations(
const TokenPartitionRange &full_range, const FormatStyle &vstyle) {
return ExtractAlignablePartitionGroups(
@@ -1681,8 +1698,11 @@
{NodeEnum::kStructUnionMemberList, &AlignStructUnionMembers},
{NodeEnum::kActualParameterByNameList, &AlignActualNamedParameters},
{NodeEnum::kPortActualList, &AlignActualNamedPorts},
+ // module/interface bodies
{NodeEnum::kModuleItemList, &AlignModuleItems},
{NodeEnum::kGenerateItemList, &AlignModuleItems},
+ // package bodies
+ {NodeEnum::kPackageItemList, &AlignModuleItems},
{NodeEnum::kFormalParameterList, &AlignParameterDeclarations},
{NodeEnum::kClassItems, &AlignClassItems},
// various case-like constructs:
diff --git a/verible/verilog/formatting/format-style-init.cc b/verible/verilog/formatting/format-style-init.cc
index 0fa91af..2dde818 100644
--- a/verible/verilog/formatting/format-style-init.cc
+++ b/verible/verilog/formatting/format-style-init.cc
@@ -105,13 +105,20 @@
ABSL_FLAG(AlignmentPolicy, named_port_alignment,
AlignmentPolicy::kInferUserIntent,
"Format named port connections: {align,flush-left,preserve,infer}");
-ABSL_FLAG(
- AlignmentPolicy, module_net_variable_alignment, //
- AlignmentPolicy::kInferUserIntent,
- "Format net/variable declarations: {align,flush-left,preserve,infer}");
+ABSL_FLAG(AlignmentPolicy, module_net_variable_alignment,
+ AlignmentPolicy::kInferUserIntent,
+ "Format net/variable declarations in module, generate, "
+ "interface, and package bodies: {align,flush-left,preserve,infer}");
ABSL_FLAG(AlignmentPolicy, formal_parameters_alignment,
AlignmentPolicy::kInferUserIntent,
- "Format formal parameters: {align,flush-left,preserve,infer}");
+ "Format formal parameters in module/interface/class headers "
+ "(inside #(...)): {align,flush-left,preserve,infer}");
+ABSL_FLAG(AlignmentPolicy, parameter_declaration_alignment,
+ AlignmentPolicy::kInferUserIntent,
+ "Format parameter/localparam declarations in module, generate, "
+ "interface, and package bodies "
+ "(class body parameter declarations are NOT affected): "
+ "{align,flush-left,preserve,infer}");
ABSL_FLAG(AlignmentPolicy, class_member_variable_alignment,
AlignmentPolicy::kInferUserIntent,
"Format class member variables: {align,flush-left,preserve,infer}");
@@ -123,7 +130,8 @@
"Align distribution items: {align,flush-left,preserve,infer}");
ABSL_FLAG(AlignmentPolicy, assignment_statement_alignment,
AlignmentPolicy::kInferUserIntent,
- "Format various assignments: {align,flush-left,preserve,infer}");
+ "Format various assignments in module, generate, interface, "
+ "and package bodies: {align,flush-left,preserve,infer}");
ABSL_FLAG(AlignmentPolicy, enum_assignment_statement_alignment,
AlignmentPolicy::kInferUserIntent,
"Format assignments with enums: {align,flush-left,preserve,infer}");
@@ -167,7 +175,7 @@
#define STYLE_FROM_FLAG(name) style->name = absl::GetFlag(FLAGS_##name)
- // Simply in the sequence as declared in struct FormatStyle
+ // In the same sequence as declared in struct FormatStyle
STYLE_FROM_FLAG(port_declarations_indentation);
STYLE_FROM_FLAG(port_declarations_alignment);
STYLE_FROM_FLAG(struct_union_members_alignment);
@@ -180,6 +188,7 @@
STYLE_FROM_FLAG(enum_assignment_statement_alignment);
STYLE_FROM_FLAG(formal_parameters_indentation);
STYLE_FROM_FLAG(formal_parameters_alignment);
+ STYLE_FROM_FLAG(parameter_declaration_alignment);
STYLE_FROM_FLAG(class_member_variable_alignment);
STYLE_FROM_FLAG(case_items_alignment);
STYLE_FROM_FLAG(distribution_items_alignment);
diff --git a/verible/verilog/formatting/format-style.h b/verible/verilog/formatting/format-style.h
index e73cf14..2fad329 100644
--- a/verible/verilog/formatting/format-style.h
+++ b/verible/verilog/formatting/format-style.h
@@ -25,7 +25,7 @@
namespace verilog {
namespace formatter {
-// Controls what breaks alignment groups for module items, statements,
+// Control what breaks alignment groups for module items, statements,
// and class items.
enum class AlignmentGroupBoundary {
// No additional group splitting (default, current behavior).
@@ -52,11 +52,9 @@
FormatStyle(const FormatStyle &) = default;
- /*
- * InitializeFromFlags() [format_style_init.h] provides flags that are
- * named like these fields and allow configuration on the command line.
- * So field foo here can be configured with flag --foo
- */
+ // InitializeFromFlags() [format-style-init.cc] provides flags that are
+ // named like these fields and allow configuration on the command line.
+ // So field foo here can be configured with flag --foo.
// TODO(hzeller): some of these are plural, some singular. Come up with
// a consistent scheme.
@@ -86,25 +84,37 @@
AlignmentPolicy named_port_alignment = AlignmentPolicy::kAlign;
// Control how module-local net/variable declarations are formatted.
+ // Applies in module, generate, interface, and package bodies.
// Internal tests assume these are forced to kAlign.
AlignmentPolicy module_net_variable_alignment = AlignmentPolicy::kAlign;
// Control how various assignment statements should be aligned.
+ // Applies in module, generate, interface, and package bodies.
// This covers: continuous assignment statements,
// blocking, and nonblocking assignments.
// Internal tests assume these are forced to kAlign.
AlignmentPolicy assignment_statement_alignment = AlignmentPolicy::kAlign;
- // Assignment within enumerations.
+ // Control how assignments in enumerations should be aligned.
AlignmentPolicy enum_assignment_statement_alignment = AlignmentPolicy::kAlign;
// Control indentation amount for formal parameter declarations.
IndentationStyle formal_parameters_indentation = IndentationStyle::kWrap;
- // Control how formal parameters in modules/interfaces/classes are formatted.
+ // Control how formal parameters in module/interface/class headers
+ // (inside #(...)) are formatted. For parameter/localparam
+ // declarations in module, generate, interface, and package bodies,
+ // see parameter_declaration_alignment.
// Internal tests assume these are forced to kAlign.
AlignmentPolicy formal_parameters_alignment = AlignmentPolicy::kAlign;
+ // Control how parameter/localparam declarations are formatted.
+ // Applies in module, generate, interface, and package bodies.
+ // Class body parameter declarations are not affected.
+ // For formal parameters in #(...) headers, see formal_parameters_alignment.
+ // Internal tests assume these are forced to kAlign.
+ AlignmentPolicy parameter_declaration_alignment = AlignmentPolicy::kAlign;
+
// Control how class member variables are formatted.
// Internal tests assume these are forced to kAlign.
AlignmentPolicy class_member_variable_alignment = AlignmentPolicy::kAlign;
@@ -120,7 +130,7 @@
bool port_declarations_right_align_packed_dimensions = false;
bool port_declarations_right_align_unpacked_dimensions = false;
- // Controls what breaks alignment groups for module items, statements,
+ // Control what breaks alignment groups for module items, statements,
// and class items.
AlignmentGroupBoundary alignment_group_boundary =
AlignmentGroupBoundary::kNone;
@@ -139,7 +149,7 @@
// Split with a \n end and else clauses
bool wrap_end_else_clauses = false;
- // -- Note: when adding new fields, add them in format_style_init.cc
+ // -- Note: When adding new fields, add them in format-style-init.cc
// TODO(fangism): introduce the following knobs:
//
@@ -174,12 +184,16 @@
: indentation_spaces;
}
+ // -- Note: When adding a new AlignmentPolicy field, add it here
+ // and to InitializeFromFlags in format-style-init.cc.
void ApplyToAllAlignmentPolicies(AlignmentPolicy policy) {
port_declarations_alignment = policy;
+ struct_union_members_alignment = policy;
named_parameter_alignment = policy;
named_port_alignment = policy;
module_net_variable_alignment = policy;
formal_parameters_alignment = policy;
+ parameter_declaration_alignment = policy;
class_member_variable_alignment = policy;
case_items_alignment = policy;
assignment_statement_alignment = policy;
diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc
index e885d74..32e3a91 100644
--- a/verible/verilog/formatting/formatter_test.cc
+++ b/verible/verilog/formatting/formatter_test.cc
@@ -4343,6 +4343,19 @@
" .L(L),\n"
" .W(W)\n"
") bar_t;\n"},
+ // unqualified parameterized type keeps a space before '#'
+ {"typedef dv_base_env_cov #(.CFG_T(tl_agent_env_cfg)) tl_agent_env_cov;\n",
+ "typedef dv_base_env_cov #(\n"
+ " .CFG_T(tl_agent_env_cfg)\n"
+ ") tl_agent_env_cov;\n"},
+ // ... and is inserted when absent
+ {"typedef dv_base_env_cov#(.CFG_T(tl_agent_env_cfg)) tl_agent_env_cov;\n",
+ "typedef dv_base_env_cov #(\n"
+ " .CFG_T(tl_agent_env_cfg)\n"
+ ") tl_agent_env_cov;\n"},
+ // single short parameter stays on one line
+ {"typedef my_class #(.P(P)) my_class_t;\n",
+ "typedef my_class #(.P(P)) my_class_t;\n"},
// package test cases
{"package fedex;localparam int www=3 ;endpackage : fedex\n",
@@ -19216,6 +19229,1513 @@
}
}
+// Verify kAlign behavior for body-level param/localparam declarations
+// in module and package bodies.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentBasics) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// localparam alignment in module body
+ "module m;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "localparam baaaaz = 4'b0111;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ " localparam baaaaz = 4'b0111;\n"
+ "endmodule\n"},
+ {// localparam alignment in package body
+ "package p;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "localparam baaaaz = 4'b0111;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ " localparam baaaaz = 4'b0111;\n"
+ "endpackage\n"},
+ {// parameter alignment in module body
+ "module m;\n"
+ "parameter int foo = 1;\n"
+ "parameter int barr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int foo = 1;\n"
+ " parameter int barr = 2;\n"
+ "endmodule\n"},
+ {// parameter alignment in package body
+ "package p;\n"
+ "parameter int foo = 1;\n"
+ "parameter int barrrr = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " parameter int foo = 1;\n"
+ " parameter int barrrr = 2;\n"
+ "endpackage\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify kFlushLeft behavior: body-level param/localparam declarations
+// are not aligned.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentFlushLeft) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// module body: parameters are not aligned
+ "module m;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ "endmodule\n"},
+ {// package context: flush-left
+ "package p;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ "endpackage\n"},
+ {// generate context: flush-left
+ "module m;\n"
+ "generate\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// parameter in module body: flush-left
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "parameter int HHHH = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " parameter int HHHH = 2;\n"
+ "endmodule\n"},
+ {// mixed parameter and localparam: flush-left, no alignment
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "localparam L = 4;\n"
+ "parameter int H = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " localparam L = 4;\n"
+ " parameter int H = 2;\n"
+ "endmodule\n"},
+ {// interface context: flush-left
+ "interface my_if;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endinterface\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kFlushLeft;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify kPreserve behavior: body-level param/localparam declarations
+// maintain their existing spacing.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentPreserve) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// module body: existing spacing is kept
+ "module m;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ "endmodule\n"},
+ {// package: existing flush-left spacing preserved
+ "package p;\n"
+ "localparam foo = 4'b0000;\n"
+ "localparam barr = 4'b0010;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 4'b0000;\n"
+ " localparam barr = 4'b0010;\n"
+ "endpackage\n"},
+ {// generate: existing aligned spacing preserved
+ "module m;\n"
+ "generate\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// parameter flush-left preserved
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "parameter int HHHH = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " parameter int HHHH = 2;\n"
+ "endmodule\n"},
+ {// parameter pre-aligned preserved
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "parameter int HHHH = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " parameter int HHHH = 2;\n"
+ "endmodule\n"},
+ {// interface pre-aligned preserved
+ "interface my_if;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endinterface\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kPreserve;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify kAlign behavior for param/localparam declarations across
+// interface, generate, and module body contexts, including mixed
+// param/localparam and packed dimension alignment.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentContexts) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// interface body localparam alignment
+ "interface my_if;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endinterface\n"},
+ {// interface body parameter alignment
+ "interface my_if;\n"
+ "parameter int X = 1;\n"
+ "parameter int Y_LONG = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " parameter int X = 1;\n"
+ " parameter int Y_LONG = 2;\n"
+ "endinterface\n"},
+ {// generate block localparam alignment
+ "module m;\n"
+ "generate\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// generate block parameter alignment
+ "module m;\n"
+ "generate\n"
+ "parameter int A = 1;\n"
+ "parameter int BB = 2;\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " parameter int A = 1;\n"
+ " parameter int BB = 2;\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// mixed param and net declarations form separate alignment groups
+ "module m;\n"
+ "localparam X = 1;\n"
+ "localparam YYY = 2;\n"
+ "logic clk;\n"
+ "logic rst_n;\n"
+ "localparam ZZ = 3;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam X = 1;\n"
+ " localparam YYY = 2;\n"
+ " logic clk;\n"
+ " logic rst_n;\n"
+ " localparam ZZ = 3;\n"
+ "endmodule\n"},
+ {// mixed parameter and localparam in same block
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "localparam L = 4;\n"
+ "parameter int H = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " localparam L = 4;\n"
+ " parameter int H = 2;\n"
+ "endmodule\n"},
+ {// packed dimensions are aligned
+ "module m;\n"
+ "parameter bit [7:0] X = 0;\n"
+ "parameter bit [31:0] YYYY = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter bit [ 7:0] X = 0;\n"
+ " parameter bit [31:0] YYYY = 1;\n"
+ "endmodule\n"},
+ {// multi-identifier comma-separated params are not split (no-crash)
+ "module m;\n"
+ "parameter int a=1, b=2, ccc=3;\n"
+ "localparam d=4, e=5;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int a = 1, b = 2, ccc = 3;\n"
+ " localparam d = 4, e = 5;\n"
+ "endmodule\n"},
+ {// parameter type declarations do not crash
+ "module m;\n"
+ "parameter type T = int;\n"
+ "parameter type TT_LONG = bit;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter type T = int;\n"
+ " parameter type TT_LONG = bit;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify kInferUserIntent behavior: body-level param/localparam
+// declarations infer alignment intent from existing spacing.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentInferUserIntent) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// flush-left localparams with small spacing diff: infer aligns
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endmodule\n"},
+ {// pre-aligned localparams: infer preserves alignment
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "localparam baaaaz = 3;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " localparam baaaaz = 3;\n"
+ "endmodule\n"},
+ {// flush-left params in package with small spacing diff: infer aligns
+ "package p;\n"
+ "localparam X = 1;\n"
+ "localparam YY = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam X = 1;\n"
+ " localparam YY = 2;\n"
+ "endpackage\n"},
+ {// mixed param/localparam flush-left: infer keeps flush-left
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "localparam L = 4;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = 8;\n"
+ " localparam L = 4;\n"
+ "endmodule\n"},
+ {// interface flush-left with small diff: infer aligns
+ "interface my_if;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endinterface\n",
+ "interface my_if;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endinterface\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kInferUserIntent;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify boundary behavior for param declarations with
+// kBlankLinesAndSeparatorComments: both blank lines and separator comments
+// break alignment groups.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentBoundary) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// separator comment breaks param alignment group
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// blank line breaks param alignment group
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// no separator: single alignment group (all aligned together)
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// regular comment does NOT break alignment group
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "// regular comment\n"
+ "localparam barr = 2;\n"
+ "localparam baaaaz = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " // regular comment\n"
+ " localparam barr = 2;\n"
+ " localparam baaaaz = 1;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary =
+ AlignmentGroupBoundary::kBlankLinesAndSeparatorComments;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify edge cases: unpacked dimensions, signed/unsigned, implicit types,
+// complex types, single declarations, deeply nested generate blocks.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentEdgeCases) {
+ // Use column_limit = 80 to avoid unintended line wrapping inside
+ // deeply nested generate blocks (generate-if, generate-for), which
+ // would interfere with verifying alignment behavior.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// unpacked dimension alignment
+ "module m;\n"
+ "parameter bit X [7:0] = 0;\n"
+ "parameter bit YYYY [15:0] = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter bit X [ 7:0] = 0;\n"
+ " parameter bit YYYY[15:0] = 1;\n"
+ "endmodule\n"},
+ {// param declarations with only type and default value (no idim)
+ "module m;\n"
+ "parameter int X = 0;\n"
+ "parameter int YYYY = 0;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int X = 0;\n"
+ " parameter int YYYY = 0;\n"
+ "endmodule\n"},
+ {// signed modifier with packed dimensions
+ "module m;\n"
+ "parameter bit signed [7:0] X = 0;\n"
+ "parameter bit signed [31:0] YYYY = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter bit signed [ 7:0] X = 0;\n"
+ " parameter bit signed [31:0] YYYY = 1;\n"
+ "endmodule\n"},
+ {// param with complex type (struct/typedef) — does not crash
+ "module m;\n"
+ "parameter foo_t X = foo_t'(0);\n"
+ "parameter foo_t Y_LONG = foo_t'(0);\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter foo_t X = foo_t'(0);\n"
+ " parameter foo_t Y_LONG = foo_t'(0);\n"
+ "endmodule\n"},
+ {// single param declaration (not enough for alignment group)
+ "module m;\n"
+ "localparam X = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam X = 1;\n"
+ "endmodule\n"},
+ {// params inside generate-if block
+ "module m;\n"
+ "generate\n"
+ "if (1) begin : blk\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "end\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " if (1) begin : blk\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " end\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ {// unsigned keyword with packed dimensions (complement to signed)
+ "module m;\n"
+ "parameter bit unsigned [7:0] X = 0;\n"
+ "parameter bit unsigned [31:0] YYYY = 1;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter bit unsigned [ 7:0] X = 0;\n"
+ " parameter bit unsigned [31:0] YYYY = 1;\n"
+ "endmodule\n"},
+ {// parameter with implicit type (no explicit type keyword)
+ "module m;\n"
+ "parameter X = 32;\n"
+ "parameter Y_LONG = 64;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter X = 32;\n"
+ " parameter Y_LONG = 64;\n"
+ "endmodule\n"},
+ {// params inside generate-for block
+ "module m;\n"
+ "generate\n"
+ "for (i = 0; i < 2; i++) begin : blk\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "end\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m;\n"
+ " generate\n"
+ " for (i = 0; i < 2; i++) begin : blk\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " end\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 80;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify parameter declarations inside generate-case blocks are aligned.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentGenerateCase) {
+ // Params inside generate-case need wider column limit to avoid wrapping the
+ // case label / begin block line.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// params inside generate-case block
+ "module m #(P = 0);\n"
+ "generate\n"
+ "case (P)\n"
+ "0: begin : blk\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "end\n"
+ "endcase\n"
+ "endgenerate\n"
+ "endmodule\n",
+ "module m #(\n"
+ " P = 0\n"
+ ");\n"
+ " generate\n"
+ " case (P)\n"
+ " 0: begin : blk\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " end\n"
+ " endcase\n"
+ " endgenerate\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 80;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+TEST(FormatterEndToEndTest, FormalAndBodyParamAlignmentIndependence) {
+ // Verify that --formal_parameters_alignment and
+ // --parameter_declaration_alignment act independently: setting formal params
+ // to kFlushLeft should not affect body-level param alignment, and vice versa.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// formal flush-left + body align: only body params align
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// formal align + body flush-left: only formal params align
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// both flush-left: no alignment anywhere
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// both align: both formal and body params aligned
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ };
+ // First case: formal kFlushLeft, body kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.formal_parameters_alignment = AlignmentPolicy::kFlushLeft;
+ const auto &tc = kTestCases[0];
+ VLOG(1) << "code-to-format:\n" << tc.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // Second case: formal kAlign, body kFlushLeft
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kFlushLeft;
+ style.formal_parameters_alignment = AlignmentPolicy::kAlign;
+ const auto &tc = kTestCases[1];
+ VLOG(1) << "code-to-format:\n" << tc.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // Third case: both kFlushLeft
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kFlushLeft;
+ style.formal_parameters_alignment = AlignmentPolicy::kFlushLeft;
+ const auto &tc = kTestCases[2];
+ VLOG(1) << "code-to-format:\n" << tc.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // Fourth case: both kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.formal_parameters_alignment = AlignmentPolicy::kAlign;
+ const auto &tc = kTestCases[3];
+ VLOG(1) << "code-to-format:\n" << tc.input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+}
+
+TEST(FormatterEndToEndTest, ClassBodyParamNotAffected) {
+ // Verify that class body parameter/localparam declarations are NOT affected
+ // by --parameter_declaration_alignment, which targets only module/generate/
+ // package/interface body-level params.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// class body params are not affected
+ "class c;\n"
+ "parameter int X = 1;\n"
+ "parameter int YYYY = 2;\n"
+ "localparam foo = 3;\n"
+ "localparam barrrr = 4;\n"
+ "endclass\n",
+ "class c;\n"
+ " parameter int X = 1;\n"
+ " parameter int YYYY = 2;\n"
+ " localparam foo = 3;\n"
+ " localparam barrrr = 4;\n"
+ "endclass\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+TEST(FormatterEndToEndTest, PackageBodyVariableAlignment) {
+ // Verify that moving kPackageItemList to kTabularAlignment in tree-unwrapper
+ // enables net/variable declaration alignment in package bodies via
+ // module_net_variable_alignment.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// net/variable declaration alignment in package body
+ "package p;\n"
+ "logic [7:0] a;\n"
+ "logic [31:0] bb;\n"
+ "endpackage\n",
+ "package p;\n"
+ " logic [ 7:0] a;\n"
+ " logic [31:0] bb;\n"
+ "endpackage\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.module_net_variable_alignment = AlignmentPolicy::kAlign;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentApplyToAll) {
+ // Verify that ApplyToAllAlignmentPolicies affects
+ // parameter_declaration_alignment for all four alignment policies.
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// kAlign aligns identifiers and = signs across declarations
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barrrr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barrrr = 2;\n"
+ "endmodule\n"},
+ {// kFlushLeft: declarations remain flush-left
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barrrr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barrrr = 2;\n"
+ "endmodule\n"},
+ {// kPreserve: existing spacing is kept
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barrrr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barrrr = 2;\n"
+ "endmodule\n"},
+ {// kInferUserIntent: large identifier width difference (3 cols)
+ // suggests flush-left intent, no alignment.
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barrrr = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barrrr = 2;\n"
+ "endmodule\n"},
+ };
+ // kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.ApplyToAllAlignmentPolicies(AlignmentPolicy::kAlign);
+ const auto &tc = kTestCases[0];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kFlushLeft
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.ApplyToAllAlignmentPolicies(AlignmentPolicy::kFlushLeft);
+ const auto &tc = kTestCases[1];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kPreserve
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.ApplyToAllAlignmentPolicies(AlignmentPolicy::kPreserve);
+ const auto &tc = kTestCases[2];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kInferUserIntent
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.ApplyToAllAlignmentPolicies(AlignmentPolicy::kInferUserIntent);
+ const auto &tc = kTestCases[3];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+}
+
+// Verify that alignment_group_boundary=kBlankLines treats only blank lines
+// (not separator comments) as boundary breaks for param declarations.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentBoundaryBlankLines) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// Only blank line breaks group; separator comment does not
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// Separator comment does NOT break group with kBlankLines
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary = AlignmentGroupBoundary::kBlankLines;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify that alignment_group_boundary=kNone keeps all param declarations
+// in a single alignment group regardless of blank lines or comments.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentBoundaryNone) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// Blank line does NOT break group with kNone
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// Separator comment does NOT break group with kNone
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary = AlignmentGroupBoundary::kNone;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify that alignment_group_boundary=kSeparatorComments treats only
+// separator comments (not blank lines) as boundary breaks for param
+// declarations.
+TEST(FormatterEndToEndTest,
+ ParamDeclarationAlignmentBoundarySeparatorComments) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// Separator comment breaks alignment group
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ {// Blank line does NOT break group with kSeparatorComments
+ "module m;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endmodule\n",
+ "module m;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary = AlignmentGroupBoundary::kSeparatorComments;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify that class body parameter/localparam declarations are NOT affected
+// by --parameter_declaration_alignment for the kFlushLeft, kPreserve,
+// and kInferUserIntent policies (kAlign is covered in
+// ClassBodyParamNotAffected).
+TEST(FormatterEndToEndTest, ClassBodyParamNotAffectedOtherPolicies) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// kFlushLeft: class body params remain flush-left
+ "class c;\n"
+ "parameter int X = 1;\n"
+ "parameter int YYYY = 2;\n"
+ "localparam foo = 3;\n"
+ "localparam barrrr = 4;\n"
+ "endclass\n",
+ "class c;\n"
+ " parameter int X = 1;\n"
+ " parameter int YYYY = 2;\n"
+ " localparam foo = 3;\n"
+ " localparam barrrr = 4;\n"
+ "endclass\n"},
+ {// kPreserve: parameter_declaration_alignment does not apply to class
+ // bodies; whitespace is normalized by default formatting.
+ "class c;\n"
+ "parameter int X = 1;\n"
+ "parameter int YYYY = 2;\n"
+ "endclass\n",
+ "class c;\n"
+ " parameter int X = 1;\n"
+ " parameter int YYYY = 2;\n"
+ "endclass\n"},
+ {// kInferUserIntent: class body params infer flush-left
+ "class c;\n"
+ "parameter int X = 1;\n"
+ "parameter int YYYY = 2;\n"
+ "endclass\n",
+ "class c;\n"
+ " parameter int X = 1;\n"
+ " parameter int YYYY = 2;\n"
+ "endclass\n"},
+ };
+ // kFlushLeft
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kFlushLeft;
+ const auto &tc = kTestCases[0];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kPreserve
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kPreserve;
+ const auto &tc = kTestCases[1];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // kInferUserIntent
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kInferUserIntent;
+ const auto &tc = kTestCases[2];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+}
+
+// Verify formal_parameters_alignment and parameter_declaration_alignment
+// independence for kPreserve and kInferUserIntent combinations.
+TEST(FormatterEndToEndTest,
+ FormalAndBodyParamAlignmentIndependenceOtherPolicies) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// formal kPreserve + body kAlign: only body params aligned
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// formal kAlign + body kPreserve: only formal params aligned
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ {// formal kInferUserIntent + body kAlign: only body params aligned
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ };
+ // formal kPreserve + body kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.formal_parameters_alignment = AlignmentPolicy::kPreserve;
+ const auto &tc = kTestCases[0];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // formal kAlign + body kPreserve
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kPreserve;
+ style.formal_parameters_alignment = AlignmentPolicy::kAlign;
+ const auto &tc = kTestCases[1];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+ // formal kInferUserIntent + body kAlign
+ {
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.formal_parameters_alignment = AlignmentPolicy::kInferUserIntent;
+ const auto &tc = kTestCases[2];
+ std::ostringstream stream;
+ const auto status = FormatVerilog(tc.input, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), tc.expected) << "code:\n" << tc.input;
+ }
+}
+
+// Verify parameter declarations with string, real, and function-call default
+// value types to exercise the ColumnSchemaScanner on non-standard type tokens.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentTypeVariants) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// parameter real type
+ "module m;\n"
+ "parameter real X = 1.0;\n"
+ "parameter real Y_LONG = 2.0;\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter real X = 1.0;\n"
+ " parameter real Y_LONG = 2.0;\n"
+ "endmodule\n"},
+ {// parameter string type
+ "module m;\n"
+ "parameter string s = \"hello\";\n"
+ "parameter string long_s = \"world\";\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter string s = \"hello\";\n"
+ " parameter string long_s = \"world\";\n"
+ "endmodule\n"},
+ {// parameter with function call in default value
+ "module m;\n"
+ "parameter int W = func(1, 2);\n"
+ "parameter int H_LONG = other(3);\n"
+ "endmodule\n",
+ "module m;\n"
+ " parameter int W = func(1, 2);\n"
+ " parameter int H_LONG = other(3);\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify parameter_declaration_alignment kInferUserIntent in package bodies.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentPackageInfer) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// flush-left params with small diff: infer aligns
+ "package p;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "endpackage\n"},
+ {// pre-aligned params: infer preserves
+ "package p;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "localparam baaaaz = 3;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " localparam baaaaz = 3;\n"
+ "endpackage\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kInferUserIntent;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify parameter_declaration_alignment boundary behavior in package bodies.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentPackageBoundary) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// blank line breaks alignment group in package
+ "package p;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ "\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endpackage\n"},
+ {// separator comment breaks alignment group in package
+ "package p;\n"
+ "localparam foo = 1;\n"
+ "localparam barr = 2;\n"
+ "// ============\n"
+ "localparam baaaaz = 1;\n"
+ "localparam c = 2;\n"
+ "endpackage\n",
+ "package p;\n"
+ " localparam foo = 1;\n"
+ " localparam barr = 2;\n"
+ " // ============\n"
+ " localparam baaaaz = 1;\n"
+ " localparam c = 2;\n"
+ "endpackage\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ style.alignment_group_boundary =
+ AlignmentGroupBoundary::kBlankLinesAndSeparatorComments;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify the most common default scenario: both formal_parameters_alignment
+// and parameter_declaration_alignment set to kInferUserIntent.
+TEST(FormatterEndToEndTest, BothParamAlignmentsInferUserIntent) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ {// both flush-left with small diff: infer aligns both formal and body
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barr = 0;\n"
+ "endmodule\n"},
+ {// both infer: pre-aligned formal, pre-aligned body preserved
+ "module m #(\n"
+ "int W = 2,\n"
+ "int LLLL = 4\n"
+ ");\n"
+ "localparam foo = 0;\n"
+ "localparam barrrr = 0;\n"
+ "endmodule\n",
+ "module m #(\n"
+ " int W = 2,\n"
+ " int LLLL = 4\n"
+ ");\n"
+ " localparam foo = 0;\n"
+ " localparam barrrr = 0;\n"
+ "endmodule\n"},
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kInferUserIntent;
+ style.formal_parameters_alignment = AlignmentPolicy::kInferUserIntent;
+ 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);
+ EXPECT_OK(status) << status.message();
+ EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
+ }
+}
+
+// Verify that a large multi-line comment block between parameter
+// declarations does not crash the formatter. The tree-unwrapper may
+// produce a partition structure where the comment tokens are absorbed
+// into the following parameter partition, which the alignment code
+// must handle gracefully by skipping alignment for that row.
+TEST(FormatterEndToEndTest, ParamDeclarationAlignmentCommentBlockNoCrash) {
+ // These inputs contain large multi-line comment blocks between
+ // parameter/localparam declarations. The formatter must not crash
+ // (SIGABRT). Output correctness is secondary; the formatter may
+ // fall back to preserving the original input when the partition
+ // structure prevents safe alignment.
+ const char *kInputs[] = {
+ "module m;\n"
+ "parameter int W = 8;\n"
+ "// Multi-line comment block\n"
+ "// that spans across\n"
+ "//\n"
+ "// several lines\n"
+ "// of explanatory text\n"
+ "//\n"
+ "// It contains enough\n"
+ "// lines to trigger\n"
+ "// the partition structure\n"
+ "// edge case where\n"
+ "// comment tokens are\n"
+ "// absorbed into the\n"
+ "// following parameter\n"
+ "// partition.\n"
+ "//\n"
+ "localparam int H = 2;\n"
+ "endmodule\n",
+ "package p;\n"
+ "parameter int X = 1;\n"
+ "// Long comment block\n"
+ "// with many lines\n"
+ "//\n"
+ "// of text\n"
+ "//\n"
+ "localparam int Y = 2;\n"
+ "endpackage\n",
+ };
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.parameter_declaration_alignment = AlignmentPolicy::kAlign;
+ for (const char *input : kInputs) {
+ VLOG(1) << "code-to-format:\n" << input << "<EOF>";
+ std::ostringstream stream;
+ const auto status = FormatVerilog(input, "<filename>", style, stream);
+ // The primary requirement is that the formatter does not crash
+ // (SIGABRT). Gtest will report failure if the process aborts.
+ // The partition structure may cause output format differences;
+ // the important thing is the formatter handled it gracefully.
+ }
+}
+
} // namespace
} // namespace formatter
} // namespace verilog
diff --git a/verible/verilog/formatting/token-annotator.cc b/verible/verilog/formatting/token-annotator.cc
index d4af24a..fcbbdbb 100644
--- a/verible/verilog/formatting/token-annotator.cc
+++ b/verible/verilog/formatting/token-annotator.cc
@@ -500,11 +500,23 @@
// This may be controversial or context-dependent, as parameterized
// classes often appear with method calls like:
// type#(params...)::method(...);
+ // A parameterized type in a typedef keeps the space before '#':
+ // typedef my_class #(.P(P)) my_class_t;
+ // but a package-qualified type does not, matching the existing
+ // "type#(params...)::method(...)" convention:
+ // typedef pkg::my_class#(.P(P)) my_class_t;
+ // Kept as separate IsInsideFirst() calls because MatchesTagAnyOf()
+ // only unrolls up to four tags.
+ const bool inside_unqualified_typedef =
+ left_context.IsInsideFirst({NodeEnum::kTypeDeclaration}, {}) &&
+ !left_context.IsInsideFirst({NodeEnum::kQualifiedId}, {});
+
if (left_context.DirectParentIs(NodeEnum::kUnqualifiedId) &&
!left_context.IsInsideFirst(
{NodeEnum::kInstantiationType, NodeEnum::kBindTargetInstance,
NodeEnum::kExtendsList, NodeEnum::kBraceGroup},
- {})) {
+ {}) &&
+ !inside_unqualified_typedef) {
return {0, "No space before # when direct parent is kUnqualifiedId."};
}
return {1, "Spaces before # in most other contexts."};
diff --git a/verible/verilog/formatting/tree-unwrapper.cc b/verible/verilog/formatting/tree-unwrapper.cc
index 15a0082..28f6391 100644
--- a/verible/verilog/formatting/tree-unwrapper.cc
+++ b/verible/verilog/formatting/tree-unwrapper.cc
@@ -1201,7 +1201,6 @@
// For the following constructs, always expand the view to subpartitions.
// Add a level of indentation.
case NodeEnum::kPackageImportList:
- case NodeEnum::kPackageItemList:
case NodeEnum::kInterfaceClassDeclaration:
case NodeEnum::kCasePatternItemList:
case NodeEnum::kConstraintBlockItemList:
@@ -1314,6 +1313,8 @@
case NodeEnum::kClassItems:
case NodeEnum::kModuleItemList:
case NodeEnum::kGenerateItemList:
+ // Aligns parameter, net/variable, and assignment declarations in packages.
+ case NodeEnum::kPackageItemList:
case NodeEnum::kDistributionItemList:
case NodeEnum::kEnumNameList:
case NodeEnum::kStructUnionMemberList: {
@@ -2898,7 +2899,7 @@
case NodeEnum::kConstraintBlockItemList: {
HoistOnlyChildPartition(&partition);
- // Alwyas expand constraint(s) blocks with braces inside them
+ // Always expand constraint(s) blocks with braces inside them
const auto &uwline = partition.Value();
const auto &ftokens = uwline.TokensRange();
auto found = std::find_if(ftokens.begin(), ftokens.end(),
diff --git a/verible/verilog/parser/verilog-parser_test.cc b/verible/verilog/parser/verilog-parser_test.cc
index 7c99a1e..d4f5933 100644
--- a/verible/verilog/parser/verilog-parser_test.cc
+++ b/verible/verilog/parser/verilog-parser_test.cc
@@ -1339,6 +1339,15 @@
static constexpr ParserTestCaseArray kModuleTests = {
"module modular_thing;\n"
"endmodule",
+ "module m;\n"
+ "wire logic b;\n"
+ "endmodule",
+ "module m;\n"
+ "wire bit b;\n"
+ "endmodule",
+ "module m;\n"
+ "wire logic [3:0] b;\n"
+ "endmodule",
"module semicolon_madness;;;;;\n"
"endmodule",
"module automatic modular_thing;\n"
diff --git a/verible/verilog/parser/verilog.y b/verible/verilog/parser/verilog.y
index 6a4a6f5..0263611 100644
--- a/verible/verilog/parser/verilog.y
+++ b/verible/verilog/parser/verilog.y
@@ -2111,6 +2111,9 @@
{ $$ = MakeTaggedNode(N::kDataTypeImplicitIdDimensions,
MakeDataType($1, MakePackedDimensionsNode($2)),
$3, nullptr, nullptr); }
+ | data_type_primitive delay3_or_drive_opt
+ { $$ = MakeTaggedNode(N::kDataTypeImplicitIdDimensions,
+ $1, $2, nullptr, nullptr); }
| GenericIdentifier decl_dimensions_opt delay3_or_drive_opt
{ $$ = MakeTaggedNode(N::kDataTypeImplicitIdDimensions,
MakeDataType(MakeTaggedNode(N::kLocalRoot,MakeTaggedNode(N::kUnqualifiedId,$1)), MakePackedDimensionsNode($2)),
diff --git a/verible/verilog/tools/formatter/README.md b/verible/verilog/tools/formatter/README.md
index 0dbafdb..01b2869 100644
--- a/verible/verilog/tools/formatter/README.md
+++ b/verible/verilog/tools/formatter/README.md
@@ -4,8 +4,8 @@
freshness: { owner: 'hzeller' reviewed: '2020-10-07' }
*-->
-`verible-verilog-format` is the SystemVerilog formatter tool. You can can
-get a full set of avilable flags using the `--helpfull` flag.
+`verible-verilog-format` is the SystemVerilog formatter tool. You can
+get a full set of available flags using the `--helpfull` flag.
For automatic formatting suggestions on github pull requests, there is a
[easy to integrate github action available][github-format-action].
@@ -35,7 +35,8 @@
operator.); default: 4;
Flags from verilog/formatting/format_style_init.cc:
- --assignment_statement_alignment (Format various assignments:
+ --assignment_statement_alignment (Format various assignments in
+ module, generate, interface, and package bodies:
{align,flush-left,preserve,infer}); default: infer;
--case_items_alignment (Format case items:
{align,flush-left,preserve,infer}); default: infer;
@@ -48,11 +49,13 @@
--enum_assignment_statement_alignment (Format assignments with enums:
{align,flush-left,preserve,infer}); default: infer;
--expand_coverpoints (If true, always expand coverpoints.); default: false;
- --formal_parameters_alignment (Format formal parameters:
- {align,flush-left,preserve,infer}); default: infer;
+ --formal_parameters_alignment (Format formal parameters in module/
+ interface/class headers (inside #(...)): {align,flush-left,preserve,infer});
+ default: infer;
--formal_parameters_indentation (Indent formal parameters: {indent,wrap});
default: wrap;
- --module_net_variable_alignment (Format net/variable declarations:
+ --module_net_variable_alignment (Format net/variable declarations
+ in module, generate, interface, and package bodies:
{align,flush-left,preserve,infer}); default: infer;
--named_parameter_alignment (Format named actual parameters:
{align,flush-left,preserve,infer}); default: infer;
@@ -62,6 +65,10 @@
{align,flush-left,preserve,infer}); default: infer;
--named_port_indentation (Indent named port connections: {indent,wrap});
default: wrap;
+ --parameter_declaration_alignment (Format parameter/localparam declarations
+ in module, generate, interface, and package bodies:
+ {align,flush-left,preserve,infer}); default: infer;
+ NOTE: class body parameter declarations are NOT affected.
--port_declarations_alignment (Format port declarations:
{align,flush-left,preserve,infer}); default: infer;
--port_declarations_indentation (Indent port declarations: {indent,wrap});