Align case items (statements).
Initially, this only does two-column alignment (after the colon), nothing smarter.
e.g:
case (x)
kFooo: xx = y;
kBar: y = xx;
endcase
This also fixes handling of //comments in case item lists.
Next: related cases, like case-inside, randcase, generate case...
issues #413
PiperOrigin-RevId: 331254916
diff --git a/common/formatting/align.cc b/common/formatting/align.cc
index 49c06dd..70efa81 100644
--- a/common/formatting/align.cc
+++ b/common/formatting/align.cc
@@ -454,8 +454,11 @@
decision = SpacingOptions::MustAppend;
}
}
- partition.Value().SetPartitionPolicy(
- PartitionPolicyEnum::kSuccessfullyAligned);
+ // Tag every subtree as having already been committed to alignment.
+ partition.ApplyPostOrder([](TokenPartitionTree& node) {
+ node.Value().SetPartitionPolicy(
+ PartitionPolicyEnum::kSuccessfullyAligned);
+ });
}
}
diff --git a/verilog/formatting/align.cc b/verilog/formatting/align.cc
index e147a9f..2639aea 100644
--- a/verilog/formatting/align.cc
+++ b/verilog/formatting/align.cc
@@ -155,6 +155,30 @@
return false;
}
+static bool TokenForcesLineBreak(const PreFormatToken& ftoken) {
+ switch (ftoken.TokenEnum()) {
+ case verilog_tokentype::TK_begin:
+ case verilog_tokentype::TK_fork:
+ return true;
+ }
+ return false;
+}
+
+static bool IgnoreMultilineCaseStatements(const TokenPartitionTree& partition) {
+ if (IgnoreCommentsAndPreprocessingDirectives(partition)) return true;
+
+ const auto& uwline = partition.Value();
+ const auto token_range = uwline.TokensRange();
+
+ // Scan for any tokens that would force a line break.
+ if (std::any_of(token_range.begin(), token_range.end(),
+ &TokenForcesLineBreak)) {
+ return true;
+ }
+
+ return false;
+}
+
// This class marks up token-subranges in named parameter assignments for
// alignment. e.g. ".parameter_name(value_expression)"
class ActualNamedParameterColumnSchemaScanner : public ColumnSchemaScanner {
@@ -689,6 +713,66 @@
bool new_column_after_open_bracket_ = false;
};
+// This class marks up token-subranges in case items for alignment.
+// e.g. "value1, value2: x = f(y);"
+class CaseItemColumnSchemaScanner : public ColumnSchemaScanner {
+ public:
+ CaseItemColumnSchemaScanner() = default;
+
+ bool ParentContextIsCaseItem() const {
+ return Context().DirectParentIsOneOf(
+ {NodeEnum::kCaseItem, NodeEnum::kDefaultItem});
+ }
+
+ void Visit(const SyntaxTreeNode& node) override {
+ auto tag = NodeEnum(node.Tag().tag);
+ VLOG(2) << __FUNCTION__ << ", node: " << tag << " at "
+ << TreePathFormatter(Path());
+
+ if (previous_token_was_case_colon_) {
+ if (ParentContextIsCaseItem()) {
+ ReserveNewColumn(node, FlushLeft);
+ previous_token_was_case_colon_ = false;
+ }
+ } else {
+ switch (tag) {
+ case NodeEnum::kCaseItem:
+ case NodeEnum::kDefaultItem: {
+ // Start a new column right away.
+ ReserveNewColumn(node, FlushLeft);
+ break;
+ }
+ default:
+ break;
+ }
+ }
+
+ // recursive visitation
+ TreeContextPathVisitor::Visit(node);
+ VLOG(2) << __FUNCTION__ << ", leaving node: " << tag;
+ }
+
+ void Visit(const SyntaxTreeLeaf& leaf) override {
+ VLOG(2) << __FUNCTION__ << ", leaf: " << leaf.get() << " at "
+ << TreePathFormatter(Path());
+ const int tag = leaf.get().token_enum();
+ switch (tag) {
+ case ':':
+ if (ParentContextIsCaseItem()) {
+ // mark the next node as the start of a new column
+ previous_token_was_case_colon_ = true;
+ }
+ break;
+ default:
+ break;
+ }
+ VLOG(2) << __FUNCTION__ << ", leaving leaf: " << leaf.get();
+ }
+
+ private:
+ bool previous_token_was_case_colon_ = false;
+};
+
static const verible::AlignedFormattingHandler kPortDeclarationAligner{
.extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter(
&verible::GetSubpartitionsBetweenBlankLines,
@@ -729,6 +813,14 @@
AlignmentCellScannerGenerator<ClassPropertyColumnSchemaScanner>(),
};
+static const verible::AlignedFormattingHandler kCaseItemAligner{
+ .extract_alignment_groups = verible::ExtractAlignmentGroupsAdapter(
+ &verible::GetSubpartitionsBetweenBlankLines,
+ &IgnoreMultilineCaseStatements),
+ .alignment_cell_scanner =
+ AlignmentCellScannerGenerator<CaseItemColumnSchemaScanner>(),
+};
+
struct AlignedFormattingConfiguration {
// Set of functions for driving specific code aligners.
verible::AlignedFormattingHandler handler;
@@ -794,6 +886,11 @@
[](const FormatStyle& vstyle) {
return vstyle.class_member_variable_alignment;
}}},
+ {NodeEnum::kCaseItemList,
+ {kCaseItemAligner,
+ [](const FormatStyle& vstyle) {
+ return vstyle.case_items_alignment;
+ }}},
};
const auto handler_iter = kAlignHandlers->find(NodeEnum(node->Tag().tag));
if (handler_iter == kAlignHandlers->end()) return;
diff --git a/verilog/formatting/format_style.h b/verilog/formatting/format_style.h
index f68821d..957c8b2 100644
--- a/verilog/formatting/format_style.h
+++ b/verilog/formatting/format_style.h
@@ -70,6 +70,10 @@
// Internal tests assume these are forced to kAlign.
AlignmentPolicy class_member_variable_alignment = AlignmentPolicy::kAlign;
+ // Control how case items are formatted.
+ // Internal tests assume these are forced to kAlign.
+ AlignmentPolicy case_items_alignment = AlignmentPolicy::kAlign;
+
// At this time line wrap optimization is problematic and risks ruining
// otherwise reasonable code. When set to false, this switch will make the
// formatter give-up and leave code as-is in cases where it would otherwise
@@ -117,6 +121,7 @@
module_net_variable_alignment = policy;
formal_parameters_alignment = policy;
class_member_variable_alignment = policy;
+ case_items_alignment = policy;
}
};
diff --git a/verilog/formatting/formatter_test.cc b/verilog/formatting/formatter_test.cc
index f4ecec9..bebb9bf 100644
--- a/verilog/formatting/formatter_test.cc
+++ b/verilog/formatting/formatter_test.cc
@@ -3836,6 +3836,19 @@
" end\n"
" endcase\n"
"endfunction\n"},
+ {// case statement, interleaved with comments
+ "function f; case (x) \n//c1\nState0 : a=b;//c2\n//c3\n State1 : "
+ "a=b;//c4\n//c5\n "
+ "endcase endfunction\n",
+ "function f;\n"
+ " case (x)\n"
+ " //c1\n"
+ " State0: a = b; //c2\n"
+ " //c3\n"
+ " State1: a = b; //c4\n"
+ " //c5\n"
+ " endcase\n"
+ "endfunction\n"},
{// case inside statement
"function f; case (x)inside k1 : return b; k2 : begin return b; end "
"endcase endfunction\n",
@@ -3884,7 +3897,7 @@
"endcase endfunction\n",
"function f;\n"
" case (x)\n"
- " k1: if (b) break;\n"
+ " k1: if (b) break;\n" // aligned
" default: return 2;\n"
" endcase\n"
"endfunction\n"},
@@ -3893,7 +3906,7 @@
"endcase endfunction\n",
"function f;\n"
" case (x)\n"
- " k1: break;\n"
+ " k1: break;\n" // aligned
" default: if (c) return 2;\n"
" endcase\n"
"endfunction\n"},
@@ -7152,6 +7165,106 @@
" int my_int;\n" // ... but still indent
" foo_pkg::bar_t my_bar;\n"
"endclass : cc\n"},
+
+ // case item test cases
+ {// small difference between flush-left and align, so align
+ "function f; case (x)kZZZZ :if( b )break; default :return 2;"
+ "endcase endfunction\n",
+ "function f;\n"
+ " case (x)\n"
+ " kZZZZ: if (b) break;\n" // aligned, only adds 2 spaces
+ " default: return 2;\n"
+ " endcase\n"
+ "endfunction\n"},
+ {// small error relative to flush-left, so flush-left
+ "function f; case (x)kZ :if( b )break; default :return 2;"
+ "endcase endfunction\n",
+ "function f;\n"
+ " case (x)\n"
+ " kZ: if (b) break;\n" // flush-left
+ " default: return 2;\n"
+ " endcase\n"
+ "endfunction\n"},
+ {// intentional spacing error (delta=4) induces alignment
+ "function f; case (x)kZ :if( b )break; default :return 2;"
+ "endcase endfunction\n",
+ "function f;\n"
+ " case (x)\n"
+ " kZ: if (b) break;\n"
+ " default: return 2;\n"
+ " endcase\n"
+ "endfunction\n"},
+ {// induced alignment, with ignored comments
+ "function f; case (x)kZ :if( b )break; \n//c1\n kXX: g = f; "
+ "\n//c2\ndefault :return 2;"
+ "endcase endfunction\n",
+ "function f;\n"
+ " case (x)\n"
+ " kZ: if (b) break;\n"
+ " //c1\n"
+ " kXX: g = f;\n"
+ " //c2\n"
+ " default: return 2;\n"
+ " endcase\n"
+ "endfunction\n"},
+ {// induced alignment, ignore multiline case item in the middle
+ "function f; case (x)"
+ "kZ :if( b )break; "
+ "kYY :return 2;" // excess spaces induce alignment
+ " kXXXXXXXXX: begin end" // multi-line, ignored
+ " kWWWWW: cc = 23;\n"
+ " kVVV: cd = 24;\n"
+ "endcase endfunction\n",
+ "function f;\n"
+ " case (x)\n"
+ " kZ: if (b) break;\n"
+ " kYY: return 2;\n"
+ " kXXXXXXXXX: begin\n" // separates above/below groups
+ " end\n"
+ " kWWWWW: cc = 23;\n"
+ " kVVV: cd = 24;\n" // aligned
+ " endcase\n"
+ "endfunction\n"},
+ {// induced alignment, ignore multiline case item in the middle
+ "function f; case (x)"
+ "kZ :if( b )break; "
+ "kYY :return 2;" // excess spaces induce alignment
+ " kXXXXXXXXX: if(w)begin end" // multi-line, ignored
+ " kWWWWW: cc = 23;\n"
+ " kVVV: cd = 24;\n"
+ "endcase endfunction\n",
+ "function f;\n"
+ " case (x)\n"
+ " kZ: if (b) break;\n"
+ " kYY: return 2;\n"
+ " kXXXXXXXXX:\n" // TODO(fangism): allow this to merge with if()
+ // else indent the following two lines
+ " if (w) begin\n" // separates above/below groups
+ " end\n"
+ " kWWWWW: cc = 23;\n"
+ " kVVV: cd = 24;\n" // aligned
+ " endcase\n"
+ "endfunction\n"},
+ {// induced alignment, ignore multiline case item in the middle
+ "task t; case (x)"
+ "kZ :if( b )break; "
+ "kYY :return 2;" // excess spaces induce alignment
+ " kXXXXXXXXX: fork join" // multi-line, ignored
+ " kWWWWW: cc = 23;\n"
+ " kVVV: cd = 24;\n"
+ "endcase endtask\n",
+ "task t;\n"
+ " case (x)\n"
+ " kZ: if (b) break;\n"
+ " kYY: return 2;\n"
+ " kXXXXXXXXX:\n" // TODO(fangism): allow this to merge with fork
+ // else indent the following two lines
+ " fork\n" // separates above/below groups
+ " join\n"
+ " kWWWWW: cc = 23;\n"
+ " kVVV: cd = 24;\n" // aligned
+ " endcase\n"
+ "endtask\n"},
};
// Use a fixed style.
FormatStyle style;
diff --git a/verilog/formatting/tree_unwrapper.cc b/verilog/formatting/tree_unwrapper.cc
index b6df5a0..e8feffb 100644
--- a/verilog/formatting/tree_unwrapper.cc
+++ b/verilog/formatting/tree_unwrapper.cc
@@ -528,6 +528,7 @@
case NodeEnum::kPackageItemList:
case NodeEnum::kSpecifyItemList:
case NodeEnum::kBlockItemStatementList:
+ case NodeEnum::kCaseItemList:
case NodeEnum::kConstraintExpressionList:
case NodeEnum::kConstraintBlockItemList:
LookAheadBeyondCurrentNode();
@@ -944,7 +945,6 @@
case NodeEnum::kPackageItemList:
case NodeEnum::kInterfaceClassDeclaration:
case NodeEnum::kGenerateItemList:
- case NodeEnum::kCaseItemList:
case NodeEnum::kCaseInsideItemList:
case NodeEnum::kCasePatternItemList:
case NodeEnum::kGenerateCaseItemList:
@@ -1004,6 +1004,7 @@
}
}
+ case NodeEnum::kCaseItemList:
case NodeEnum::kClassItems:
case NodeEnum::kModuleItemList: {
const int indent = suppress_indentation ? 0 : style_.indentation_spaces;
diff --git a/verilog/formatting/tree_unwrapper_test.cc b/verilog/formatting/tree_unwrapper_test.cc
index 709a719..9f33cc2 100644
--- a/verilog/formatting/tree_unwrapper_test.cc
+++ b/verilog/formatting/tree_unwrapper_test.cc
@@ -6891,6 +6891,35 @@
},
{
+ "function with case statement, with comments",
+ "function foo_case;"
+ "case (y) \n"
+ "//c1\n"
+ "k1: return 0;\n"
+ "//c2\n"
+ "k2: return 1;\n"
+ "//c3\n"
+ "endcase "
+ "endfunction",
+ FunctionDeclaration(
+ 0, FunctionHeader(0, {"function", "foo_case", ";"}),
+ FlowControl(1, L(1, {"case", "(", "y", ")"}),
+ CaseItemList(2, //
+ L(2, {"//c1"}), //
+ N(2, //
+ L(2, {"k1", ":"}), //
+ L(2, {"return", "0", ";"})),
+ L(2, {"//c2"}), //
+ N(2, //
+ L(2, {"k2", ":"}), //
+ L(2, {"return", "1", ";"})),
+ L(2, {"//c3"}) //
+ ), //
+ L(1, {"endcase"})), //
+ L(0, {"endfunction"})),
+ },
+
+ {
"function with case statements",
"function foo_case;"
"case (y) "
diff --git a/verilog/tools/formatter/verilog_format.cc b/verilog/tools/formatter/verilog_format.cc
index 188c4a4..c5572b7 100644
--- a/verilog/tools/formatter/verilog_format.cc
+++ b/verilog/tools/formatter/verilog_format.cc
@@ -157,6 +157,9 @@
ABSL_FLAG(AlignmentPolicy, class_member_variables_alignment,
AlignmentPolicy::kInferUserIntent,
"Format class member variables: {align,flush-left,preserve,infer}");
+ABSL_FLAG(AlignmentPolicy, case_items_alignment,
+ AlignmentPolicy::kInferUserIntent,
+ "Format case items: {align,flush-left,preserve,infer}");
ABSL_FLAG(bool, try_wrap_long_lines, false,
"If true, let the formatter attempt to optimize line wrapping "
@@ -236,6 +239,8 @@
absl::GetFlag(FLAGS_formal_parameters_alignment);
format_style.class_member_variable_alignment =
absl::GetFlag(FLAGS_class_member_variables_alignment);
+ format_style.case_items_alignment =
+ absl::GetFlag(FLAGS_case_items_alignment);
}
std::ostringstream stream;
