Fix formatter abort on line-continuation before aligned ports Skip alignment for partitions where non-tree tokens precede a forced wrap (e.g. // comment then `\`), and stop CHECK-failing in align.h so formatting no longer aborts (issue #2539).
diff --git a/verible/common/formatting/align.cc b/verible/common/formatting/align.cc index 9986fc7..318a76b 100644 --- a/verible/common/formatting/align.cc +++ b/verible/common/formatting/align.cc
@@ -1001,6 +1001,18 @@ auto &line = node.Value(); auto ftokens = line.TokensRange(); + // Leading non-tree tokens before a forced wrap must stay on their own + // line. Putting them in a kInline prolog cell would glue e.g. `//\` onto + // the following `input` (GitHub issue 2539). Preserve original spacing + // for the whole row instead. + if (align_actions.front().ftoken != ftokens.begin() && + align_actions.front().ftoken->before.break_decision == + SpacingOptions::kMustWrap) { + FormatUsingOriginalSpacing(TokenPartitionRange(*row, std::next(*row))); + ++row; + continue; + } + line.SetPartitionPolicy(PartitionPolicyEnum::kAlreadyFormatted); verible::TokenPartitionTree *current_cell = nullptr;
diff --git a/verible/common/formatting/align.h b/verible/common/formatting/align.h index d042ae0..5989a6f 100644 --- a/verible/common/formatting/align.h +++ b/verible/common/formatting/align.h
@@ -410,19 +410,24 @@ // Use next token as begining 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: + // Leading non-tree tokens (e.g. // comments, line-continuation `\`) cannot + // be placed into alignment cells when the first syntax-tree token must + // start a new line (SpacingOptions::kMustWrap). Including them would glue + // the leading tokens onto the origin line via kInline cells. // - // { (>>[...], (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); + // This shape often appears when a `//` comment is followed by `\` (line + // continuation) and is partitioned with the following port/declaration + // (GitHub issue 2539). Prefer leaving leading tokens out of alignment + // over CHECK-failing. Callers should also ignore such partitions (see + // PartitionHasLeadingTokensBeforeForcedWrap) so ApplyAlignment never + // builds a kInline prolog for them. A future improvement could split the + // comment into its own ignored row so the following declaration can still + // participate in alignment. + if (!leading_tokens.empty() && first_tree_token_it != ftokens.end() && + first_tree_token_it->before.break_decision == + SpacingOptions::kMustWrap) { + leading_tokens.set_end(leading_tokens.begin()); + } } else { // All tokens are passed as leading leading_tokens.set_end(ftokens.end());
diff --git a/verible/verilog/formatting/align.cc b/verible/verilog/formatting/align.cc index 1f7f472..e307047 100644 --- a/verible/verilog/formatting/align.cc +++ b/verible/verilog/formatting/align.cc
@@ -147,6 +147,32 @@ b == AlignmentGroupBoundary::kBlankLinesAndSeparatorComments; } +// True when non-tree tokens (e.g. // comments, line-continuation `\`) precede +// the origin, and the first origin token must start a new line. Aligning such +// partitions would glue the leading tokens onto the origin line via kInline +// cells (GitHub issue 2539). Leave them out of alignment instead. +static bool PartitionHasLeadingTokensBeforeForcedWrap( + const TokenPartitionTree &partition) { + const auto &uwline = partition.Value(); + const verible::Symbol *origin = uwline.Origin(); + if (origin == nullptr) return false; + + const auto ftokens = uwline.TokensRange(); + if (ftokens.empty()) return false; + + const verible::SyntaxTreeLeaf *first_leaf = verible::GetLeftmostLeaf(*origin); + if (first_leaf == nullptr) return false; + + const verible::TokenInfo &first_tree_token = first_leaf->get(); + auto ftoken_it = ftokens.begin(); + while (ftoken_it != ftokens.end() && + *(ftoken_it->token) != first_tree_token) { + ++ftoken_it; + } + if (ftoken_it == ftokens.begin() || ftoken_it == ftokens.end()) return false; + return ftoken_it->before.break_decision == verible::SpacingOptions::kMustWrap; +} + static bool IgnoreCommentsAndPreprocessingDirectives( const TokenPartitionTree &partition) { const auto &uwline = partition.Value(); @@ -159,6 +185,8 @@ // ignore lines containing only comments if (TokensAreAllCommentsOrAttributes(token_range)) return true; + if (PartitionHasLeadingTokensBeforeForcedWrap(partition)) return true; + // ignore partitions belonging to preprocessing directives return IsPreprocessorKeyword( verilog_tokentype(token_range.front().TokenEnum())); @@ -199,6 +227,8 @@ return true; } + if (PartitionHasLeadingTokensBeforeForcedWrap(partition)) return true; + // ignore nested structs/unions if (verible::FindFirstSubtree( partition.Value().Origin(), [](const Symbol &symbol) {
diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc index e885d74..25a733d 100644 --- a/verible/verilog/formatting/formatter_test.cc +++ b/verible/verilog/formatting/formatter_test.cc
@@ -19216,6 +19216,32 @@ } } +// Regression for https://github.com/chipsalliance/verible/issues/2539: +// A // comment followed by a line-continuation `\` before aligned ports must +// not abort in align.h, and must keep the comment on its own line. +TEST(FormatterEndToEndTest, PortListCommentWithLineContinuationDoesNotAbort) { + static constexpr std::string_view kInput = + "module m (\n" + "//\\\n" + "input a\n" + ",input b\n" + ");\n" + "endmodule\n"; + FormatStyle style; + std::ostringstream stream; + const auto status = FormatVerilog(kInput, "<filename>", style, stream); + EXPECT_OK(status) << status.message(); + const std::string out = stream.str(); + // Must not glue the line-continuation onto the following port declaration. + EXPECT_THAT(out, testing::Not(testing::HasSubstr("//\\ input"))); + EXPECT_THAT(out, testing::HasSubstr("//\\")); + EXPECT_THAT(out, testing::HasSubstr("input a")); + EXPECT_THAT(out, testing::HasSubstr("input b")); + EXPECT_THAT(out, testing::HasSubstr("endmodule")); + // Comment line and first port remain on separate lines. + EXPECT_THAT(out, testing::ContainsRegex(R"(//\\\n\s*input a)")); +} + } // namespace } // namespace formatter } // namespace verilog