Merge pull request #2535 from EylonKrause/fix/preprocess-truncated-callable-macro
[preprocessor] Fix crash/hang on truncated callable macro invocation
diff --git a/.github/bin/smoke-test.sh b/.github/bin/smoke-test.sh
index d10251b..5532187 100755
--- a/.github/bin/smoke-test.sh
+++ b/.github/bin/smoke-test.sh
@@ -29,17 +29,13 @@
# crash Verible, we're good.
###
-# Suppress '... aborted' messages bash would print when a tool crashes.
-# Comment out to see syntax errors in bash while working on script.
-exec 2>/dev/null
-
set -u # Be strict: only allow using a variable after it is assigned
BAZEL_BUILD_OPTIONS="-c opt"
TMPDIR="${TMPDIR:-/tmp}"
readonly BASE_TEST_DIR="${TMPDIR}/test/verible-smoke-test"
-readonly DEFAULT_HASH_FILE="$(dirname $0)/smoke-projects.hashes"
+readonly PROJECT_HASHES_FILE="$(dirname $0)/smoke-projects.hashes"
# Write log files to this directory
readonly SMOKE_LOGGING_DIR="${SMOKE_LOGGING_DIR:-$BASE_TEST_DIR/error-logs}"
@@ -82,7 +78,6 @@
#
# There are some known issues which are all recorded in the associative
# array below, mapping them to Verible issue tracker numbers.
-readonly PROJECT_HASHES_FILE="${1:-${DEFAULT_HASH_FILE}}"
if [ ! -f "${PROJECT_HASHES_FILE}" ]; then
echo "Project hashes file not found: ${PROJECT_HASHES_FILE}"
@@ -359,8 +354,55 @@
return ${result}
}
+# --- main
+
+KEEP_LOGS=0
+VERBOSE=0
+PROJECT_FILTER=""
+
+while [[ $# -gt 0 ]]; do
+ case "$1" in
+ --keep-logs)
+ KEEP_LOGS=1
+ shift
+ ;;
+ --verbose|-v)
+ VERBOSE=1
+ shift
+ ;;
+ --filter=*)
+ PROJECT_FILTER="${1#*=}"
+ shift
+ ;;
+ --filter|-f)
+ if [[ $# -lt 2 ]]; then
+ echo "Error: $1 requires an argument." >&2
+ exit 1
+ fi
+ PROJECT_FILTER="$2"
+ shift 2
+ ;;
+ -h|--help)
+ echo "Usage: $0 [--keep-logs] [--verbose] [--filter=<name>]"
+ exit 0
+ ;;
+ *)
+ echo "Unknown option: $1" >&2
+ echo "Usage: $0 [--keep-logs] [--verbose] [--filter=<name>]" >&2
+ exit 1
+ ;;
+ esac
+done
+
+if [ ${VERBOSE} -eq 0 ]; then
+ # Suppress '... aborted' messages bash would print when a tool crashes.
+ exec 2>/dev/null
+fi
+
mkdir -p "${BASE_TEST_DIR}"
-trap 'rm -rf -- "${BASE_TEST_DIR}"' EXIT
+if [ ${KEEP_LOGS} -eq 0 ]; then
+ trap 'rm -rf -- "${BASE_TEST_DIR}"' EXIT
+fi
status_sum=0
@@ -378,6 +420,9 @@
while read -r git_hash git_project _; do
[[ -z "${git_hash}" || "${git_hash}" =~ ^# ]] && continue
PROJECT_NAME="$(basename "${git_project}")"
+ if [[ -n "${PROJECT_FILTER}" && "${PROJECT_NAME}" != *"${PROJECT_FILTER}"* ]]; then
+ continue
+ fi
PROJECT_DIR="${BASE_TEST_DIR}/${PROJECT_NAME}"
( git clone "${git_project}" "${PROJECT_DIR}" && git -C "${PROJECT_DIR}" checkout -q "${git_hash}" ) 2>/dev/null &
done < "${PROJECT_HASHES_FILE}"
@@ -389,6 +434,9 @@
while read -r git_hash git_project _; do
[[ -z "${git_hash}" || "${git_hash}" =~ ^# ]] && continue
PROJECT_NAME="$(basename "${git_project}")"
+ if [[ -n "${PROJECT_FILTER}" && "${PROJECT_NAME}" != *"${PROJECT_FILTER}"* ]]; then
+ continue
+ fi
PROJECT_DIR="${BASE_TEST_DIR}/${PROJECT_NAME}"
# Already cloned above
@@ -411,7 +459,7 @@
echo "There were a total of ${status_sum} mismatches"
# Let's see if there are any issues that are fixed in the meantime.
-if [ "${#KnownIssue[@]}" -ne 0 ]; then
+if [ -z "${PROJECT_FILTER}" ] && [ "${#KnownIssue[@]}" -ne 0 ]; then
echo "::warning ::There are ${#KnownIssue[@]} tool/file combinations, that no longer fail"
declare -A DistinctIssues
for key in "${!KnownIssue[@]}"; do
@@ -423,7 +471,10 @@
for issue_id in "${!DistinctIssues[@]}"; do
echo " 🐞 ${ISSUE_PREFIX}/${issue_id}"
done
- echo
+fi
+
+if [ ${KEEP_LOGS} -ne 0 ]; then
+ echo "Logs and project files kept in ${BASE_TEST_DIR} (error logs: ${SMOKE_LOGGING_DIR})"
fi
exit ${status_sum}
diff --git a/README.md b/README.md
index b813b76..047fb20 100644
--- a/README.md
+++ b/README.md
@@ -172,8 +172,9 @@
Verible's code base is written in C++.
-To build, you need the [bazel] build system (Min version 7) and a C++20
-compatible compiler.
+To build, you need the [bazel] build system (get it from
+[bazel install][bazel-install] if not already on your system) and a
+C++20 compatible compiler.
Use your package manager to install the dependencies; on a system with
the nix package manager simply run `nix-shell` to get a build environment.
@@ -199,6 +200,8 @@
bazel build -c opt --config=create_static_linked_executables //...
```
+See [Installation](#installation-1) for install.
+
### Optionally using local flex/bison for build
Flex and Bison, that are needed for the parser generation, are compiled as part
@@ -283,6 +286,7 @@
[UHDM] format. If you are interested in collaborating, contact us.
[bazel]: https://bazel.build/
+[bazel-install]: https://bazel.build/install
[SV-LRM]: https://ieeexplore.ieee.org/document/8299595
[lint-rule-list]: https://chipsalliance.github.io/verible/lint.html
[github-lint-action]: https://github.com/chipsalliance/verible-linter-action
diff --git a/shell.nix b/shell.nix
index 85163eb..408340b 100644
--- a/shell.nix
+++ b/shell.nix
@@ -8,6 +8,17 @@
#verible_used_stdenv = pkgs.gcc15Stdenv;
#verible_used_stdenv = pkgs.clang19Stdenv;
bazel = pkgs.bazel_8;
+
+
+ userNixPath = ./user.nix; # optional user config
+ userPackages =
+ if builtins.pathExists userNixPath
+ then
+ let loaded = import userNixPath;
+ in if builtins.isFunction loaded
+ then loaded { inherit pkgs; }
+ else loaded
+ else [];
in
verible_used_stdenv.mkDerivation {
name = "verible-build-environment";
@@ -40,7 +51,7 @@
llvmPackages_22.clang-tools # for clang-tidy
llvmPackages_19.clang-tools # for clang-format
- ];
+ ] ++ userPackages;
shellHook = ''
# clang tidy: use latest.
export CLANG_TIDY=${pkgs.llvmPackages_22.clang-tools}/bin/clang-tidy
diff --git a/verible/common/util/container-proxy_test.cc b/verible/common/util/container-proxy_test.cc
index 5e8220f..4e4cd75 100644
--- a/verible/common/util/container-proxy_test.cc
+++ b/verible/common/util/container-proxy_test.cc
@@ -874,13 +874,14 @@
const int new_capacity = initial_capacity + 42;
this->proxy.reserve(new_capacity);
- EXPECT_EQ(this->proxy.capacity(), new_capacity);
- EXPECT_EQ(this->container.capacity(), new_capacity);
+ EXPECT_GE(this->proxy.capacity(), new_capacity);
+ EXPECT_EQ(this->proxy.capacity(), this->container.capacity());
+ const int capacity_after_first_reserve = this->proxy.capacity();
const int lower_capacity = 1;
this->proxy.reserve(lower_capacity);
- EXPECT_EQ(this->proxy.capacity(), new_capacity);
- EXPECT_EQ(this->container.capacity(), new_capacity);
+ EXPECT_EQ(this->proxy.capacity(), capacity_after_first_reserve);
+ EXPECT_EQ(this->container.capacity(), capacity_after_first_reserve);
}
} // namespace
diff --git a/verible/verilog/analysis/checkers/BUILD b/verible/verilog/analysis/checkers/BUILD
index bd3a98b..4681607 100644
--- a/verible/verilog/analysis/checkers/BUILD
+++ b/verible/verilog/analysis/checkers/BUILD
@@ -817,6 +817,7 @@
"//verible/common/analysis:syntax-tree-lint-rule",
"//verible/common/analysis/matcher",
"//verible/common/analysis/matcher:bound-symbol-manager",
+ "//verible/common/text:config-utils",
"//verible/common/text:symbol",
"//verible/common/text:syntax-tree-context",
"//verible/common/text:token-info",
@@ -826,7 +827,9 @@
"//verible/verilog/CST:verilog-nonterminals",
"//verible/verilog/analysis:descriptions",
"//verible/verilog/analysis:lint-rule-registry",
+ "@abseil-cpp//absl/status",
"@abseil-cpp//absl/strings",
+ "@re2",
],
alwayslink = 1,
)
@@ -1821,6 +1824,7 @@
"//verible/common/text:symbol",
"//verible/common/text:syntax-tree-context",
"//verible/common/text:token-info",
+ "//verible/common/util:container-util",
"//verible/verilog/CST:port",
"//verible/verilog/CST:verilog-matchers",
"//verible/verilog/analysis:descriptions",
diff --git a/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc b/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc
index c8822ce..e678ad5 100644
--- a/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc
+++ b/verible/verilog/analysis/checkers/generate-label-prefix-rule.cc
@@ -14,12 +14,17 @@
#include "verible/verilog/analysis/checkers/generate-label-prefix-rule.h"
+#include <memory>
+#include <string>
#include <string_view>
-#include "absl/strings/match.h"
+#include "absl/status/status.h"
+#include "absl/strings/str_cat.h"
+#include "re2/re2.h"
#include "verible/common/analysis/lint-rule-status.h"
#include "verible/common/analysis/matcher/bound-symbol-manager.h"
#include "verible/common/analysis/matcher/matcher.h"
+#include "verible/common/text/config-utils.h"
#include "verible/common/text/symbol.h"
#include "verible/common/text/syntax-tree-context.h"
#include "verible/common/text/token-info.h"
@@ -38,22 +43,36 @@
// Register the lint rule
VERILOG_REGISTER_LINT_RULE(GenerateLabelPrefixRule);
-static constexpr std::string_view kMessage =
- "All generate block labels must start with g_ or gen_";
+static constexpr std::string_view kDefaultStyleRegex = "(g_|gen_).*";
-// TODO(fangism): and be lower_snake_case?
-// TODO(fangism): generalize to a configurable pattern and
-// rename this class/rule to GenerateLabelNamingStyle?
+GenerateLabelPrefixRule::GenerateLabelPrefixRule()
+ : style_regex_(
+ std::make_unique<re2::RE2>(kDefaultStyleRegex, re2::RE2::Quiet)) {}
const LintRuleDescriptor &GenerateLabelPrefixRule::GetDescriptor() {
static const LintRuleDescriptor d{
.name = "generate-label-prefix",
.topic = "generate-constructs",
- .desc = "Checks that every generate block label starts with g_ or gen_.",
+ .desc =
+ "Checks that every generate block label matches the regex defined by "
+ "style_regex. The default regex requires labels to start with g_ or "
+ "gen_. Refer to https://github.com/chipsalliance/verible/tree/master/"
+ "verilog/tools/lint#readme for more detail on verible regex "
+ "patterns.",
+ // NOLINTNEXTLINE(misc-include-cleaner)
+ .param = {{"style_regex", std::string(kDefaultStyleRegex),
+ "A regex used to check generate label style."}},
};
return d;
}
+std::string GenerateLabelPrefixRule::CreateViolationMessage() const {
+ return absl::StrCat(
+ "Generate block label does not match the naming convention defined by "
+ "regex pattern: ",
+ style_regex_->pattern());
+}
+
// Matches begin statements
static const Matcher &BlockMatcher() {
static const Matcher matcher(NodekGenerateBlock());
@@ -84,15 +103,24 @@
}
if (label != nullptr) {
- if (!(absl::StartsWith(label->text(), "g_") ||
- absl::StartsWith(label->text(), "gen_"))) {
- violations_.insert(verible::LintViolation(*label, kMessage, context));
+ if (!RE2::FullMatch(label->text(), *style_regex_)) {
+ violations_.insert(verible::LintViolation(
+ *label, CreateViolationMessage(), context));
}
}
}
}
}
+// NOLINTNEXTLINE(misc-include-cleaner)
+absl::Status GenerateLabelPrefixRule::Configure(
+ std::string_view configuration) {
+ using verible::config::SetRegex;
+ absl::Status s = verible::ParseNameValues(
+ configuration, {{"style_regex", SetRegex(&style_regex_)}});
+ return s;
+}
+
verible::LintRuleStatus GenerateLabelPrefixRule::Report() const {
return verible::LintRuleStatus(violations_, GetDescriptor());
}
diff --git a/verible/verilog/analysis/checkers/generate-label-prefix-rule.h b/verible/verilog/analysis/checkers/generate-label-prefix-rule.h
index 9090cf3..e18df9c 100644
--- a/verible/verilog/analysis/checkers/generate-label-prefix-rule.h
+++ b/verible/verilog/analysis/checkers/generate-label-prefix-rule.h
@@ -15,8 +15,13 @@
#ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_GENERATE_LABEL_PREFIX_RULE_H_
#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_GENERATE_LABEL_PREFIX_RULE_H_
+#include <memory>
#include <set>
+#include <string>
+#include <string_view>
+#include "absl/status/status.h"
+#include "re2/re2.h"
#include "verible/common/analysis/lint-rule-status.h"
#include "verible/common/analysis/syntax-tree-lint-rule.h"
#include "verible/common/text/symbol.h"
@@ -27,20 +32,28 @@
namespace analysis {
// GenerateLabelPrefixRule checks that all generate block labels start
-// with g_ or gen_
+// with g_ or gen_ (configurable via style_regex)
class GenerateLabelPrefixRule : public verible::SyntaxTreeLintRule {
public:
using rule_type = verible::SyntaxTreeLintRule;
+ GenerateLabelPrefixRule();
+
static const LintRuleDescriptor &GetDescriptor();
+ std::string CreateViolationMessage() const;
+
void HandleSymbol(const verible::Symbol &symbol,
const verible::SyntaxTreeContext &context) final;
verible::LintRuleStatus Report() const final;
+ absl::Status Configure(std::string_view configuration) final;
+
private:
std::set<verible::LintViolation> violations_;
+
+ std::unique_ptr<re2::RE2> style_regex_;
};
} // namespace analysis
diff --git a/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc b/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc
index b46bfe4..cb43501 100644
--- a/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc
+++ b/verible/verilog/analysis/checkers/generate-label-prefix-rule_test.cc
@@ -27,6 +27,7 @@
namespace {
using verible::LintTestCase;
+using verible::RunConfiguredLintTestCases;
using verible::RunLintTestCases;
TEST(GenerateLabelPrefixRuleTest, Various) {
@@ -229,6 +230,40 @@
RunLintTestCases<VerilogAnalyzer, GenerateLabelPrefixRule>(kTestCases);
}
+TEST(GenerateLabelPrefixRuleTest, CustomRegex) {
+ const std::initializer_list<LintTestCase> kTestCases = {
+ {"module m;\n"
+ "generate\n"
+ "if (1) begin : my_custom_label\n"
+ "end\n"
+ "endgenerate\nendmodule\n"},
+ {"module m;\n"
+ "generate\n"
+ "for (genvar i=0; i<5; i++) begin : my_custom_label\n"
+ "end\n"
+ "endgenerate\nendmodule\n"},
+ };
+ RunConfiguredLintTestCases<VerilogAnalyzer, GenerateLabelPrefixRule>(
+ kTestCases, "style_regex:my_.*");
+
+ const std::initializer_list<LintTestCase> kFailCases = {
+ {"module m;\n"
+ "generate\n"
+ "if (1) begin : ",
+ {SymbolIdentifier, "g_label"},
+ "\nend\n"
+ "endgenerate\nendmodule\n"},
+ {"module m;\n"
+ "generate\n"
+ "if (1) begin : ",
+ {SymbolIdentifier, "gen_label"},
+ "\nend\n"
+ "endgenerate\nendmodule\n"},
+ };
+ RunConfiguredLintTestCases<VerilogAnalyzer, GenerateLabelPrefixRule>(
+ kFailCases, "style_regex:my_.*");
+}
+
} // namespace
} // namespace analysis
} // namespace verilog
diff --git a/verible/verilog/analysis/checkers/port-name-suffix-rule.cc b/verible/verilog/analysis/checkers/port-name-suffix-rule.cc
index 85a0db3..ffecef3 100644
--- a/verible/verilog/analysis/checkers/port-name-suffix-rule.cc
+++ b/verible/verilog/analysis/checkers/port-name-suffix-rule.cc
@@ -28,6 +28,7 @@
#include "verible/common/text/symbol.h"
#include "verible/common/text/syntax-tree-context.h"
#include "verible/common/text/token-info.h"
+#include "verible/common/util/container-util.h"
#include "verible/verilog/CST/port.h"
#include "verible/verilog/CST/verilog-matchers.h"
#include "verible/verilog/analysis/descriptions.h"
@@ -90,10 +91,16 @@
{"output", {"o", "no", "po"}},
{"inout", {"io", "nio", "pio"}}};
- // At this point it is guaranteed that the direction will be set to
- // one of the expected values (used as keys in the map above).
- // Therefore checking the suffix like this is safe
- return suffixes.at(direction).count(suffix) == 1;
+ // `direction` is usually one of the map keys, but the grammar also permits a
+ // `ref` port direction, which has no suffix convention. FindWithDefault looks
+ // the direction up with an empty-set fallback, so an unknown direction (e.g.
+ // `ref`) has no required suffixes and is treated as "correct" (no violation),
+ // consistent with Violation() which also ignores non-input/output/inout
+ // directions.
+ static const std::set<std::string_view> kNoConvention;
+ const std::set<std::string_view> &valid =
+ verible::container::FindWithDefault(suffixes, direction, kNoConvention);
+ return valid.empty() || valid.count(suffix) == 1;
}
void PortNameSuffixRule::HandleSymbol(const Symbol &symbol,
@@ -113,8 +120,11 @@
absl::StrSplit(name, '_', absl::SkipEmpty());
if (name_parts.size() < 2) {
- // No suffix at all
+ // No suffix at all. This also covers an all-underscore name (e.g. "_"),
+ // for which SkipEmpty leaves name_parts empty; return here so the
+ // name_parts.back() access below is not reached on an empty vector.
Violation(direction, token, context);
+ return;
}
if (!IsSuffixCorrect(name_parts.back(), direction)) {
diff --git a/verible/verilog/analysis/checkers/port-name-suffix-rule_test.cc b/verible/verilog/analysis/checkers/port-name-suffix-rule_test.cc
index 69f18f4..db69d71 100644
--- a/verible/verilog/analysis/checkers/port-name-suffix-rule_test.cc
+++ b/verible/verilog/analysis/checkers/port-name-suffix-rule_test.cc
@@ -57,6 +57,9 @@
{"module t (input bit name_i); endmodule;"},
{"module t (output bit abc_o); endmodule;"},
{"module t (inout bit xyz_io); endmodule;"},
+ // A `ref` port has no suffix convention and must not be flagged (and must
+ // not crash the rule via std::map::at).
+ {"module t (ref logic data_x); endmodule;"},
{"module t (input logic name_i,\n"
"output logic abc_o,\n"
"inout logic xyz_io,\n"
@@ -90,6 +93,10 @@
{"module t (output logic ", {kToken, "_o"}, "); endmodule;"},
{"module t (inout logic ", {kToken, "_io"}, "); endmodule;"},
+ // An all-underscore name splits (SkipEmpty) to an empty parts list; it
+ // must report a suffix violation, not dereference an empty vector.
+ {"module t (input logic ", {kToken, "_"}, "); endmodule;"},
+
{"module t (input logic ", {kToken, "namei"}, "); endmodule;"},
{"module t (input logic ", {kToken, "nam_ei"}, "); endmodule;"},
{"module t (input logic ", {kToken, "name_o"}, "); endmodule;"},
diff --git a/verible/verilog/analysis/verilog-equivalence.cc b/verible/verilog/analysis/verilog-equivalence.cc
index f8a47d0..68c8c98 100644
--- a/verible/verilog/analysis/verilog-equivalence.cc
+++ b/verible/verilog/analysis/verilog-equivalence.cc
@@ -89,6 +89,20 @@
return IsUnlexed(verilog_tokentype(token.token_enum()));
}
+// MacroIdentifier vs MacroIdItem depends only on whether the macro ends the
+// line (see POST_MACRO_ID in verilog.lex). Spelling-equal macros are
+// format-equivalent across that reclassification.
+static bool AreSpellingEqualLineEndingMacros(const TokenInfo &left,
+ const TokenInfo &right) {
+ const auto is_line_ending_macro = [](int token_enum) {
+ return token_enum == verilog_tokentype::MacroIdentifier ||
+ token_enum == verilog_tokentype::MacroIdItem;
+ };
+ return is_line_ending_macro(left.token_enum()) &&
+ is_line_ending_macro(right.token_enum()) &&
+ left.text() == right.text();
+}
+
DiffStatus VerilogLexicallyEquivalent(
std::string_view left, std::string_view right,
const std::function<bool(const verible::TokenInfo &)> &remove_predicate,
@@ -167,11 +181,17 @@
DiffStatus diff_status = DiffStatus::kEquivalent;
auto recursive_comparator = [&](const TokenSequence::const_iterator l,
const TokenSequence::const_iterator r) {
+ // Some token enums differ only by surrounding whitespace (e.g. whether a
+ // macro or ')' ends a line). Treat those pairs as matching enums when the
+ // spelling is unchanged so FormatEquivalent tolerates re-wrapping.
+ const bool whitespace_dependent_macro_enum_match =
+ ((l->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
+ r->text() == ")") ||
+ (r->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
+ l->text() == ")") ||
+ AreSpellingEqualLineEndingMacros(*l, *r));
if (l->token_enum() != r->token_enum() &&
- !((l->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
- r->text() == ")") ||
- (r->token_enum() == verilog_tokentype::MacroCallCloseToEndLine &&
- l->text() == ")"))) {
+ !whitespace_dependent_macro_enum_match) {
if (errstream != nullptr) {
*errstream << "Mismatched token enums. got: ";
token_printer(*l, *errstream);
@@ -258,6 +278,9 @@
(r.text() == ")"))) {
return true;
}
+ if (AreSpellingEqualLineEndingMacros(l, r)) {
+ return true;
+ }
return l.EquivalentWithoutLocation(r);
},
errstream);
diff --git a/verible/verilog/analysis/verilog-equivalence_test.cc b/verible/verilog/analysis/verilog-equivalence_test.cc
index 90b0ee1..1cb9436 100644
--- a/verible/verilog/analysis/verilog-equivalence_test.cc
+++ b/verible/verilog/analysis/verilog-equivalence_test.cc
@@ -255,6 +255,21 @@
}
}
+// MacroIdentifier vs MacroIdItem depends on whether the macro ends the line.
+TEST(FormatEquivalentTest, EquivalenceOfMacroIdentifierAndMacroIdItem) {
+ const char *kSameSpelling[] = {
+ "assign x = f(`TOKEN);\n",
+ "assign x = f(\n`TOKEN\n);\n",
+ };
+ ExpectCompareWithErrstream(FormatEquivalent, DiffStatus::kEquivalent,
+ kSameSpelling[0], kSameSpelling[1]);
+
+ // Different macro names remain different.
+ ExpectCompareWithErrstream(FormatEquivalent, DiffStatus::kDifferent,
+ "assign x = f(`TOKEN);\n",
+ "assign x = f(`OTHER);\n");
+}
+
TEST(FormatEquivalentTest, DiagnosticMismatch) {
const char *kTestCases[] = {
"module foo;\n",
diff --git a/verible/verilog/formatting/format-style-init.cc b/verible/verilog/formatting/format-style-init.cc
index 2dde818..a6d010b 100644
--- a/verible/verilog/formatting/format-style-init.cc
+++ b/verible/verilog/formatting/format-style-init.cc
@@ -140,6 +140,13 @@
"Use compact binary expressions inside indexing / bit selection "
"operators");
+ABSL_FLAG(bool, class_parameter_space, false,
+ "If true, keep/insert a space before '#' in a class"
+ "parameterized typedef, e.g. \"typedef my_class #(.P(P)) "
+ "my_class_t;\". If false (default), no space is inserted, "
+ "matching the convention used for IEEE parameterized class "
+ "instantiations, e.g. \"type#(params...)::method(...)\".");
+
ABSL_FLAG(bool, wrap_end_else_clauses, false,
"Split end and else keywords into separate lines");
@@ -197,6 +204,7 @@
STYLE_FROM_FLAG(try_wrap_long_lines);
STYLE_FROM_FLAG(expand_coverpoints);
STYLE_FROM_FLAG(compact_indexing_and_selections);
+ STYLE_FROM_FLAG(class_parameter_space);
STYLE_FROM_FLAG(wrap_end_else_clauses);
STYLE_FROM_FLAG(alignment_group_boundary);
diff --git a/verible/verilog/formatting/format-style.h b/verible/verilog/formatting/format-style.h
index 2fad329..55347c5 100644
--- a/verible/verilog/formatting/format-style.h
+++ b/verible/verilog/formatting/format-style.h
@@ -146,6 +146,9 @@
// Compact binary expressions inside indexing / bit selection operators
bool compact_indexing_and_selections = true;
+ // Keep/insert a space before '#' in a parameterized class typedef
+ bool class_parameter_space = false;
+
// Split with a \n end and else clauses
bool wrap_end_else_clauses = false;
diff --git a/verible/verilog/formatting/formatter_test.cc b/verible/verilog/formatting/formatter_test.cc
index 5205a01..13a9fb4 100644
--- a/verible/verilog/formatting/formatter_test.cc
+++ b/verible/verilog/formatting/formatter_test.cc
@@ -4478,19 +4478,42 @@
" .L(L),\n"
" .W(W)\n"
") bar_t;\n"},
- // unqualified parameterized type keeps a space before '#'
+ // By default (class_parameter_space == false), no space before '#'
{"typedef dv_base_env_cov #(.CFG_T(tl_agent_env_cfg)) tl_agent_env_cov;\n",
- "typedef dv_base_env_cov #(\n"
+ "typedef dv_base_env_cov#(\n"
" .CFG_T(tl_agent_env_cfg)\n"
") tl_agent_env_cov;\n"},
- // ... and is inserted when absent
{"typedef dv_base_env_cov#(.CFG_T(tl_agent_env_cfg)) tl_agent_env_cov;\n",
- "typedef dv_base_env_cov #(\n"
+ "typedef dv_base_env_cov#(\n"
" .CFG_T(tl_agent_env_cfg)\n"
") tl_agent_env_cov;\n"},
// single short parameter stays on one line
{"typedef my_class #(.P(P)) my_class_t;\n",
- "typedef my_class #(.P(P)) my_class_t;\n"},
+ "typedef my_class#(.P(P)) my_class_t;\n"},
+
+ // let declarations each stay on their own line
+ {"module t;\n"
+ "let OFF = 4;\n"
+ "let UNIQUE = 32;\n"
+ "let PP(a) = 30 + a;\n"
+ "endmodule\n",
+ "module t;\n"
+ " let OFF = 4;\n"
+ " let UNIQUE = 32;\n"
+ " let PP(a) = 30 + a;\n"
+ "endmodule\n"},
+
+ // let declarations each stay on their own line
+ {"module t;\n"
+ "let OFF = 4;\n"
+ "let UNIQUE = 32;\n"
+ "let PP(a) = 30 + a;\n"
+ "endmodule\n",
+ "module t;\n"
+ " let OFF = 4;\n"
+ " let UNIQUE = 32;\n"
+ " let PP(a) = 30 + a;\n"
+ "endmodule\n"},
// package test cases
{"package fedex;localparam int www=3 ;endpackage : fedex\n",
@@ -4707,6 +4730,15 @@
" for (int i = 0; i < f(m); i--) begin\n"
" end\n"
"endfunction\n"},
+ {// for loop with an attribute instance in the initializer.
+ // Regression: this used to abort with a CHECK failure while reshaping
+ // the kForSpec partitions when an attribute appears in the header.
+ "module m; initial for(int i=0(* a *);i<4;i++) x=i; endmodule",
+ "module m;\n"
+ " initial\n"
+ " for (int i = 0 (* a *); i < 4; i++)\n"
+ " x = i;\n"
+ "endmodule\n"},
{// forever loop
"function\nvoid\tforevah;forever begin "
"++k\n;end endfunction\n",
@@ -18788,6 +18820,50 @@
}
}
+static constexpr FormatterTestCase
+ kSpaceBeforeHashInUnqualifiedTypedefTestCases[] = {
+ // unqualified parameterized type keeps a space before '#'
+ {"typedef dv_base_env_cov #(.CFG_T(tl_agent_env_cfg)) "
+ "tl_agent_env_cov;\n",
+ "typedef dv_base_env_cov #(\n"
+ " .CFG_T(tl_agent_env_cfg)\n"
+ ") tl_agent_env_cov;\n"},
+ // ... and is inserted when absent
+ {"typedef dv_base_env_cov#(.CFG_T(tl_agent_env_cfg)) "
+ "tl_agent_env_cov;\n",
+ "typedef dv_base_env_cov #(\n"
+ " .CFG_T(tl_agent_env_cfg)\n"
+ ") tl_agent_env_cov;\n"},
+ // single short parameter stays on one line
+ {"typedef my_class #(.P(P)) my_class_t;\n",
+ "typedef my_class #(.P(P)) my_class_t;\n"},
+ // package-qualified types are unaffected (no space before '#')
+ {"typedef foo_pkg::baz_t#(.L(L), .W(W)) bar_t;\n",
+ "typedef foo_pkg::baz_t#(\n"
+ " .L(L),\n"
+ " .W(W)\n"
+ ") bar_t;\n"},
+};
+
+TEST(FormatterEndToEndTest, SpaceBeforeHashInUnqualifiedTypedefTestCases) {
+ // Use a fixed style.
+ FormatStyle style;
+ style.column_limit = 40;
+ style.indentation_spaces = 2;
+ style.wrap_spaces = 4;
+ style.class_parameter_space = true;
+
+ for (const auto &test_case : kSpaceBeforeHashInUnqualifiedTypedefTestCases) {
+ 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;
+ }
+}
+
static constexpr FormatterTestCase kFunctionCallsWithComments[] = {
{// no comments
"module foo;\n"
@@ -19380,6 +19456,24 @@
}
}
+// Regression for https://github.com/chipsalliance/verible/issues/2544:
+// Wrapping a $bits(...)'(...) cast may leave `MACRO at EOL, reclassifying
+// MacroIdentifier as MacroIdItem. FormatEquivalent must accept that, and
+// formatting must still pass verification.
+TEST(FormatterEndToEndTest, MacroBeforeCloseParenFormatEquivalent) {
+ static constexpr std::string_view kInput =
+ "module m;\n"
+ " assign result_value = $bits(result_value)'( "
+ "compare_bytes(input_data[DATA_WIDTH_INT-1:0], "
+ "input_datak[STROBE_WIDTH_INT-1:0], `TOKEN_BYTE) );\n"
+ "endmodule\n";
+ FormatStyle style;
+ std::ostringstream stream;
+ const auto status = FormatVerilog(kInput, "<filename>", style, stream);
+ EXPECT_OK(status) << status.message();
+ EXPECT_THAT(stream.str(), testing::HasSubstr("`TOKEN_BYTE"));
+}
+
// Regression for https://github.com/chipsalliance/verible/issues/2542:
// Continuation EOL comments after a wrapped assign must keep a stable column
// across re-format (convergence).
diff --git a/verible/verilog/formatting/token-annotator.cc b/verible/verilog/formatting/token-annotator.cc
index def1349..ebaeb21 100644
--- a/verible/verilog/formatting/token-annotator.cc
+++ b/verible/verilog/formatting/token-annotator.cc
@@ -510,7 +510,8 @@
// This may be controversial or context-dependent, as parameterized
// classes often appear with method calls like:
// type#(params...)::method(...);
- // A parameterized type in a typedef keeps the space before '#':
+ // If style.class_parameter_space is enabled, a
+ // parameterized type in a typedef keeps the space before '#':
// typedef my_class #(.P(P)) my_class_t;
// but a package-qualified type does not, matching the existing
// "type#(params...)::method(...)" convention:
@@ -518,6 +519,7 @@
// Kept as separate IsInsideFirst() calls because MatchesTagAnyOf()
// only unrolls up to four tags.
const bool inside_unqualified_typedef =
+ style.class_parameter_space &&
left_context.IsInsideFirst({NodeEnum::kTypeDeclaration}, {}) &&
!left_context.IsInsideFirst({NodeEnum::kQualifiedId}, {});
diff --git a/verible/verilog/formatting/tree-unwrapper.cc b/verible/verilog/formatting/tree-unwrapper.cc
index a6601ac..2286f8c 100644
--- a/verible/verilog/formatting/tree-unwrapper.cc
+++ b/verible/verilog/formatting/tree-unwrapper.cc
@@ -801,6 +801,7 @@
case NodeEnum::kPreprocessorUndef:
case NodeEnum::kTFPortDeclaration:
case NodeEnum::kTypeDeclaration:
+ case NodeEnum::kLetDeclaration:
case NodeEnum::kNetTypeDeclaration:
case NodeEnum::kForwardDeclaration:
case NodeEnum::kInterfaceClassMethod:
@@ -2990,10 +2991,15 @@
auto &children = partition.Children();
const auto iter1 = std::find_if(children.begin(), children.end(),
PartitionStartsWithSemicolon);
- CHECK(iter1 != children.end());
+ // An attribute instance ((* ... *)) in the for-loop header can change
+ // the partition structure so that the expected semicolon-leading
+ // partitions are not present. When they are missing, leave the
+ // partitions unreshaped rather than aborting (avoids a CHECK-failure
+ // crash on attributed/unusual for-headers).
+ if (iter1 == children.end()) break;
const auto iter2 =
std::find_if(iter1 + 1, children.end(), PartitionStartsWithSemicolon);
- CHECK(iter2 != children.end());
+ if (iter2 == children.end()) break;
const int dist1 = std::distance(children.begin(), iter1);
const int dist2 = std::distance(children.begin(), iter2);
VLOG(4) << "kForSpec got ';' at child " << dist1 << " and " << dist2;
diff --git a/verible/verilog/tools/formatter/README.md b/verible/verilog/tools/formatter/README.md
index 01b2869..a3ffb45 100644
--- a/verible/verilog/tools/formatter/README.md
+++ b/verible/verilog/tools/formatter/README.md
@@ -42,6 +42,11 @@
{align,flush-left,preserve,infer}); default: infer;
--class_member_variable_alignment (Format class member variables:
{align,flush-left,preserve,infer}); default: infer;
+ --class_parameter_space (If true, keep/insert a space
+ before '#' in a class parameterized typedef, e.g. "typedef
+ my_class #(.P(P)) my_class_t;". If false (default), no space is
+ inserted, matching the IEEE convention used for parameterized class
+ instantiations, e.g. "type#(params...)::method(...)".); default: false;
--compact_indexing_and_selections (Use compact binary expressions inside
indexing / bit selection operators); default: true;
--distribution_items_alignment (Align distribution items:
