Merge pull request #2612 from hzeller/feature-20260918-limit-tidy
Bring clang-tidy back to zero
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/.github/bin/check-potential-problems.sh b/.github/bin/check-potential-problems.sh
index 1af0f5f..c7d9f97 100755
--- a/.github/bin/check-potential-problems.sh
+++ b/.github/bin/check-potential-problems.sh
@@ -108,11 +108,9 @@
EXIT_CODE=1
fi
-# Need to skip this until https://github.com/chipsalliance/verible/issues/2435
-# resolved.
-#if [ -e .bazelversion ]; then
-# echo "Don't use .bazelversion. It is a poorly implemented bazel feature that does not support semantic versioning. Instead, make the repo work with all currently active bazel versions."
-# EXIT_CODE=1
-#fi
+if [ -e .bazelversion ]; then
+ echo "Don't use .bazelversion. It is a poorly implemented bazel feature that does not support semantic versioning. Instead, make the repo work with all currently active bazel versions."
+ EXIT_CODE=1
+fi
exit "${EXIT_CODE}"
diff --git a/.github/bin/run-build-cleaner.sh b/.github/bin/run-build-cleaner.sh
index d5b03f7..e372646 100755
--- a/.github/bin/run-build-cleaner.sh
+++ b/.github/bin/run-build-cleaner.sh
@@ -17,19 +17,25 @@
set -e
BANT=$($(dirname $0)/get-bant-path.sh)
+BAZEL=bazel
# Run build so that we have all dependencies downloaded and genrules
# materialized.
-bazel build -k --remote_download_outputs=all ...
+for f in abseil-cpp nlohmann_json protobuf re2 rules_flex zlib googletest ; do
+ "${BAZEL}" fetch --repo "@$f" > /dev/null 2>&1
+done
-if "${BANT}" -q dwyu ... ; then
+"${BAZEL}" build -k --remote_download_outputs=all \
+ $(${BANT} genrule-outputs ... -c2) > /dev/null 2>&1
+
+if "${BANT}" dwyu $@; then
echo "Dependencies ok." >&2
else
cat >&2 <<EOF
Build dependency issues found, the following one-liner will fix it. Amend PR.
-source <(.github/bin/run-build-cleaner.sh)
+source <(.github/bin/run-build-cleaner.sh $@)
EOF
exit 1
fi
diff --git a/.github/workflows/verible-ci.yml b/.github/workflows/verible-ci.yml
index 802a7a3..31a7c21 100644
--- a/.github/workflows/verible-ci.yml
+++ b/.github/workflows/verible-ci.yml
@@ -51,7 +51,7 @@
echo "--- check formatting ---"
CLANG_FORMAT=clang-format-19 ./.github/bin/run-format.sh --show-diff
echo "--- check build dependencies ---"
- ./.github/bin/run-build-cleaner.sh
+ ./.github/bin/run-build-cleaner.sh ...
echo "--- check potential problems ---"
./.github/bin/check-potential-problems.sh
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 6b42b60..29fe61e 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) {
@@ -281,6 +281,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) {
