Merge pull request #2585 from EylonKrause/fix-forspec-attribute-crash
formatter: avoid crash on attribute instance in for-loop header
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/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..2151eaa 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,
)
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/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 f9bb19e..bd0f775 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",
@@ -18797,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"
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 c373590..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:
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:
