Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions verible/verilog/formatting/formatter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19216,6 +19216,78 @@ TEST(FormatterEndToEndTest,
}
}

// Regression for https://github.com/chipsalliance/verible/issues/2540:
// Trailing EOL comment after `end` before `else if` must not change whether
// the else-if assignment stays on one line across re-format (convergence).
TEST(FormatterEndToEndTest, EndElseIfWithEOLCommentConverges) {
static constexpr FormatterTestCase kTestCases[] = {
{// Comment on its own line between end and else if
"module m;\n"
" always_comb begin\n"
" case (state)\n"
" STATE_A: begin\n"
" if (cond_aaaa) next_state_value = STATE_B;\n"
" else if (cond_bbbb) begin\n"
" next_state_value = STATE_B;\n"
" end\n"
" // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n"
" else if (cond_cccc) next_state_value = STATE_C;\n"
" end\n"
" endcase\n"
" end\n"
"endmodule\n",
"module m;\n"
" always_comb begin\n"
" case (state)\n"
" STATE_A: begin\n"
" if (cond_aaaa) next_state_value = STATE_B;\n"
" else if (cond_bbbb) begin\n"
" next_state_value = STATE_B;\n"
" end // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n"
" else if (cond_cccc) next_state_value = STATE_C;\n"
" end\n"
" endcase\n"
" end\n"
"endmodule\n"},
{// Same construct with comment already on the end line
"module m;\n"
" always_comb begin\n"
" case (state)\n"
" STATE_A: begin\n"
" if (cond_aaaa) next_state_value = STATE_B;\n"
" else if (cond_bbbb) begin\n"
" next_state_value = STATE_B;\n"
" end // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n"
" else if (cond_cccc) next_state_value = STATE_C;\n"
" end\n"
" endcase\n"
" end\n"
"endmodule\n",
"module m;\n"
" always_comb begin\n"
" case (state)\n"
" STATE_A: begin\n"
" if (cond_aaaa) next_state_value = STATE_B;\n"
" else if (cond_bbbb) begin\n"
" next_state_value = STATE_B;\n"
" end // xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\n"
" else if (cond_cccc) next_state_value = STATE_C;\n"
" end\n"
" endcase\n"
" end\n"
"endmodule\n"},
};
FormatStyle style; // default column_limit (100)
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;
}
}

} // namespace
} // namespace formatter
} // namespace verilog
17 changes: 17 additions & 0 deletions verible/verilog/formatting/tree-unwrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1816,13 +1816,30 @@ static void HoistOnlyChildPartition(TokenPartitionTree *partition) {
}
}

// True if any token in this leaf partition is an EOL comment.
static bool PartitionContainsEOLComment(const TokenPartitionTree &partition) {
for (const auto &token : partition.Value().TokensRange()) {
if (token.TokenEnum() == verilog_tokentype::TK_EOL_COMMENT) return true;
}
return false;
}

static void PushEndIntoElsePartition(TokenPartitionTree *partition_ptr) {
// Then combine 'end' with the following 'else' ...
// Do not flatten, so that if- and else- clauses can make formatting
// decisions independently from each other.
auto &partition = *partition_ptr;
auto &if_clause_partition = partition.Children().front();
auto *end_partition = &RightmostDescendant(if_clause_partition);
// When 'end' carries a trailing EOL comment, 'else' must start on the next
// line (see token annotator: comment before else => MustWrap). Merging
// end+comment into the else-if header makes fit-else-expand treat the
// header as wider than the eventual formatted line, which wraps the
// else-if body on re-format and fails convergence (GitHub issue 2540).
if (PartitionContainsEOLComment(*end_partition)) {
VLOG(4) << "end has EOL comment, skip merge into else";
return;
}
auto *end_parent = verible::MergeLeafIntoNextLeaf(end_partition);
// if moving leaf results in any singleton partitions, hoist.
if (end_parent != nullptr) {
Expand Down
Loading