diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc index e885d7427..08791827b 100644 --- a/verible/verilog/formatting/formatter_test.cc +++ b/verible/verilog/formatting/formatter_test.cc @@ -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 << ""; + std::ostringstream stream; + const auto status = + FormatVerilog(test_case.input, "", style, stream); + EXPECT_OK(status) << status.message(); + EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input; + } +} + } // namespace } // namespace formatter } // namespace verilog diff --git a/verible/verilog/formatting/tree-unwrapper.cc b/verible/verilog/formatting/tree-unwrapper.cc index 15a008280..e24aaabf4 100644 --- a/verible/verilog/formatting/tree-unwrapper.cc +++ b/verible/verilog/formatting/tree-unwrapper.cc @@ -1816,6 +1816,14 @@ 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 @@ -1823,6 +1831,15 @@ static void PushEndIntoElsePartition(TokenPartitionTree *partition_ptr) { 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) {