Merge pull request #2613 from hzeller/feature-20260918-simplify-dwyu
Reduce the preparation work on cold project for build cleaning
diff --git a/.bazelrc b/.bazelrc
index 301b564..dc2e33d 100644
--- a/.bazelrc
+++ b/.bazelrc
@@ -1,6 +1,8 @@
# bazel < 7 needs explicit enabling of bzlmod dependencies.
build --enable_bzlmod
+test --test_output=errors
+
# Enable support for absl types like string_view in gtest.
build --define="absl=1"
diff --git a/verible/common/formatting/align.cc b/verible/common/formatting/align.cc
index 74bc432..9f77014 100644
--- a/verible/common/formatting/align.cc
+++ b/verible/common/formatting/align.cc
@@ -1001,6 +1001,20 @@
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. Master's scanner already omits those
+ // leading tokens from column bids; this keeps ApplyAlignment from
+ // still inlining them.
+ 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/verilog/formatting/align.cc b/verible/verilog/formatting/align.cc
index 6640adb..24696c7 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_class_package_test.cc b/verible/verilog/formatting/formatter_class_package_test.cc
index 0c5e067..ee45c51 100644
--- a/verible/verilog/formatting/formatter_class_package_test.cc
+++ b/verible/verilog/formatting/formatter_class_package_test.cc
@@ -96,6 +96,18 @@
"endinterface\n",
},
{
+ // Keep space before explicit modport port name
+ "interface\tfoo ;"
+ "modport mp1(input .a(sig), output .b(sig));"
+ "endinterface",
+ "interface foo;\n"
+ " modport mp1(\n"
+ " input .a(sig),\n"
+ " output .b(sig)\n"
+ " );\n"
+ "endinterface\n",
+ },
+ {
// interface with long modport port names
"interface\tfoo_if ;"
"modport mp1\t( output a_long_output, input detailed_input_name);"
diff --git a/verible/verilog/formatting/formatter_issue_regression_test.cc b/verible/verilog/formatting/formatter_issue_regression_test.cc
index 8f1fcef..9490a88 100644
--- a/verible/verilog/formatting/formatter_issue_regression_test.cc
+++ b/verible/verilog/formatting/formatter_issue_regression_test.cc
@@ -314,6 +314,28 @@
EXPECT_EQ(stream.str(), test_case.expected) << "code:\n" << test_case.input;
}
}
+
+// 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 FormatterTestCase kTestCases[] = {
+ {"module m (\n"
+ "//\\\n"
+ "input a\n"
+ ",input b\n"
+ ");\n"
+ "endmodule\n",
+ "module m (\n"
+ " //\\\n"
+ " input a\n"
+ " , input b\n"
+ ");\n"
+ "endmodule\n"},
+ };
+ FormatStyle style; // default column_limit (100)
+ RunFormatterTestCases(style, kTestCases);
+}
} // namespace
} // namespace formatter
} // namespace verilog
diff --git a/verible/verilog/formatting/formatter_module_test.cc b/verible/verilog/formatting/formatter_module_test.cc
index f6e142b..c61da86 100644
--- a/verible/verilog/formatting/formatter_module_test.cc
+++ b/verible/verilog/formatting/formatter_module_test.cc
@@ -3050,6 +3050,40 @@
RunFormatterTestCases40(kModuleFormatterTestCases);
}
+TEST(FormatterEndToEndTest, TernaryInsideSubscriptExpression_issue2597) {
+ static constexpr FormatterTestCase kTestCases[] = {
+ // Outside subscript
+ {"module foo ();\n"
+ "assign a = b > 1'h0 ? 1'h0 : c;\n"
+ "endmodule\n",
+
+ "module foo ();\n"
+ " assign a = b > 1'h0 ? 1'h0 : c;\n"
+ "endmodule\n"},
+
+ // Inside subscript.
+ {"module foo ();\n"
+ "assign a = some_array[b > 1'h0 ? 1'h0 : c];\n"
+ "endmodule\n",
+
+ "module foo ();\n"
+ " assign a = some_array[b > 1'h0 ? 1'h0 : c];\n"
+ "endmodule\n"},
+ };
+
+ FormatStyle style;
+ style.indentation_spaces = 2;
+ 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);
+ // Require these test cases to be valid.
+ 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/token-annotator.cc b/verible/verilog/formatting/token-annotator.cc
index fc6d2f0..d66042f 100644
--- a/verible/verilog/formatting/token-annotator.cc
+++ b/verible/verilog/formatting/token-annotator.cc
@@ -117,7 +117,7 @@
return context.IsInsideFirst(
{NodeEnum::kDimensionScalar, NodeEnum::kDimensionRange,
NodeEnum::kDimensionSlice, NodeEnum::kCycleDelayRange},
- {});
+ {NodeEnum::kConditionExpression}); // exclude
}
static bool IsAnySemicolon(const PreFormatToken &ftoken) {
@@ -282,6 +282,12 @@
// TODO(fangism): Never insert trailing spaces before a newline.
+ // Modport port name separator, e.g. "input .a("
+ if (right.TokenEnum() == '.' &&
+ right_context.IsInside(NodeEnum::kModportSimplePort)) {
+ return {1, "Space before modport explicit port name '.'"};
+ }
+
// Hierarchy examples: "a.b", "a::b"
if (left.format_token_enum == FormatTokenType::hierarchy ||
right.format_token_enum == FormatTokenType::hierarchy) {
diff --git a/verible/verilog/formatting/token-annotator_test.cc b/verible/verilog/formatting/token-annotator_test.cc
index 2adf936..c0f7e86 100644
--- a/verible/verilog/formatting/token-annotator_test.cc
+++ b/verible/verilog/formatting/token-annotator_test.cc
@@ -2753,6 +2753,24 @@
{1, SpacingOptions::kUndecided},
},
+ // Modport explicit port name, e.g. "input .a(sig)"
+ {
+ DefaultStyle,
+ {TK_input, "input"},
+ {'.', "."},
+ {/* any context */},
+ {NodeEnum::kModportSimplePort},
+ {1, SpacingOptions::kUndecided},
+ },
+ {
+ DefaultStyle,
+ {TK_output, "output"},
+ {'.', "."},
+ {/* any context */},
+ {NodeEnum::kModportSimplePort},
+ {1, SpacingOptions::kUndecided},
+ },
+
// Handle '->' as a unary prefix expression.
{
DefaultStyle,
@@ -5501,6 +5519,31 @@
{NodeEnum::kUnpackedDimensions},
{1, SpacingOptions::kPreserve},
},
+ {
+ // [b > 1'h0 ? 1'h0 : c] : space around '>' in ternary inside
+ // subscript
+ DefaultStyle,
+ verilog_tokentype::SymbolIdentifier,
+ "b",
+ " ", // 1 space originally
+ '>',
+ ">",
+ {NodeEnum::kDimensionScalar, NodeEnum::kConditionExpression},
+ {NodeEnum::kDimensionScalar, NodeEnum::kConditionExpression},
+ {1, SpacingOptions::kUndecided},
+ },
+ {
+ // [b > 1 ? 1 : c] : space before '?' in ternary inside subscript
+ DefaultStyle,
+ verilog_tokentype::TK_DecNumber,
+ "1",
+ " ", // 1 space originally
+ '?',
+ "?",
+ {NodeEnum::kDimensionScalar, NodeEnum::kConditionExpression},
+ {NodeEnum::kDimensionScalar, NodeEnum::kConditionExpression},
+ {1, SpacingOptions::kUndecided},
+ },
};
int test_index = 0;
for (const auto &test_case : kTestCases) {
