Skip to content

Commit

Permalink
Adding function to parse and return a regex for lint rule configuration
Browse files Browse the repository at this point in the history
  • Loading branch information
sconwayaus committed Jun 14, 2024
1 parent f915e22 commit 82728e7
Show file tree
Hide file tree
Showing 14 changed files with 317 additions and 53 deletions.
30 changes: 18 additions & 12 deletions .github/bin/smoke-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ readonly TEST_GIT_PROJECTS="https://github.com/lowRISC/ibex \
https://github.com/rsd-devel/rsd \
https://github.com/syntacore/scr1 \
https://github.com/olofk/serv \
https://github.com/bespoke-silicon-group/basejump_stl"
https://github.com/bespoke-silicon-group/basejump_stl \
https://github.com/gtaylormb/opl3_fpga"

##
# Some of the files in the projects will have issues.
Expand Down Expand Up @@ -131,33 +132,33 @@ declare -A ExpectedFailCount

ExpectedFailCount[syntax:ibex]=14
ExpectedFailCount[lint:ibex]=14
ExpectedFailCount[project:ibex]=199
ExpectedFailCount[preprocessor:ibex]=373
ExpectedFailCount[project:ibex]=211
ExpectedFailCount[preprocessor:ibex]=385

ExpectedFailCount[syntax:opentitan]=36
ExpectedFailCount[lint:opentitan]=36
ExpectedFailCount[project:opentitan]=825
ExpectedFailCount[preprocessor:opentitan]=2337
ExpectedFailCount[project:opentitan]=800
ExpectedFailCount[preprocessor:opentitan]=2238

ExpectedFailCount[syntax:sv-tests]=77
ExpectedFailCount[lint:sv-tests]=76
ExpectedFailCount[project:sv-tests]=187
ExpectedFailCount[preprocessor:sv-tests]=139

ExpectedFailCount[syntax:caliptra-rtl]=24
ExpectedFailCount[lint:caliptra-rtl]=24
ExpectedFailCount[project:caliptra-rtl]=333
ExpectedFailCount[syntax:caliptra-rtl]=25
ExpectedFailCount[lint:caliptra-rtl]=25
ExpectedFailCount[project:caliptra-rtl]=335
ExpectedFailCount[preprocessor:caliptra-rtl]=777

ExpectedFailCount[syntax:Cores-VeeR-EH2]=2
ExpectedFailCount[lint:Cores-VeeR-EH2]=2
ExpectedFailCount[project:Cores-VeeR-EH2]=42
ExpectedFailCount[preprocessor:Cores-VeeR-EH2]=43

ExpectedFailCount[syntax:cva6]=6
ExpectedFailCount[lint:cva6]=6
ExpectedFailCount[project:cva6]=92
ExpectedFailCount[preprocessor:cva6]=129
ExpectedFailCount[syntax:cva6]=7
ExpectedFailCount[lint:cva6]=7
ExpectedFailCount[project:cva6]=86
ExpectedFailCount[preprocessor:cva6]=124

ExpectedFailCount[syntax:uvm]=1
ExpectedFailCount[lint:uvm]=1
Expand Down Expand Up @@ -212,6 +213,11 @@ ExpectedFailCount[project:basejump_stl]=581
ExpectedFailCount[formatter:basejump_stl]=1
ExpectedFailCount[preprocessor:basejump_stl]=615

ExpectedFailCount[syntax:opl3_fpga]=2
ExpectedFailCount[lint:opl3_fpga]=2
ExpectedFailCount[project:opl3_fpga]=4
ExpectedFailCount[preprocessor:opl3_fpga]=4

# Ideally, we expect all tools to process all files with a zero exit code.
# However, that is not always the case, so we document the current
# state, so that we can see regressions.
Expand Down
43 changes: 42 additions & 1 deletion .github/workflows/verible-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,47 @@ jobs:
name: "diag"
path: "**/plot_*.svg"

RunBantBuildCleaneer:
# Running http://bant.build/ to check all dependencies in BUILD files.
runs-on: ubuntu-24.04
steps:
- name: Checkout code
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Build Project genrules
run: |
# Fetch all dependency and run genrules for bant to see every file
# that makes it into the compile.
bazel fetch ...
bazel build \
//common/analysis:command-file-lexer \
//common/lsp:jcxxgen-testfile-gen \
//common/lsp:lsp-protocol-gen \
//common/util:version-header \
//verilog/CST:verilog-nonterminals-foreach-gen \
//verilog/parser:gen-verilog-token-enum \
//verilog/parser:verilog-lex \
//verilog/parser:verilog-parse-interface \
//verilog/parser:verilog-y \
//verilog/parser:verilog-y-final \
//verilog/tools/kythe:verilog-extractor-indexing-fact-type-foreach-gen
- name: Get Bant
run: |
# TODO: provide this as action where we simply say with version=...
VERSION="v0.1.3"
STATIC_VERSION="bant-${VERSION}-linux-static-x86_64"
wget "https://github.com/hzeller/bant/releases/download/${VERSION}/${STATIC_VERSION}.tar.gz"
tar xvzf "${STATIC_VERSION}.tar.gz"
mkdir -p bin
ln -sf ../"${STATIC_VERSION}/bin/bant" bin/
bin/bant -V
- name: Run bant build-cleaner
run: |
bin/bant dwyu ...
Check:
container: ubuntu:jammy
Expand Down Expand Up @@ -435,7 +476,7 @@ jobs:
apt -qq -y install npm nodejs
WindowsBuild:
runs-on: windows-2022
runs-on: windows-2019
steps:

- uses: actions/checkout@v3
Expand Down
3 changes: 0 additions & 3 deletions common/analysis/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ cc_library(
"//common/util:algorithm",
"//common/util:logging",
"@com_google_absl//absl/strings:string_view",
"@com_google_googletest//:gtest", # for library testonly
],
)

Expand Down Expand Up @@ -266,7 +265,6 @@ cc_library(
"//common/text:text-structure",
"//common/util:logging",
"@com_google_absl//absl/strings:string_view",
"@com_google_googletest//:gtest", # for library testonly
],
)

Expand Down Expand Up @@ -322,7 +320,6 @@ cc_library(
"//common/text:text-structure",
"//common/util:logging",
"@com_google_absl//absl/strings:string_view",
"@com_google_googletest//:gtest", # for library testonly
],
)

Expand Down
2 changes: 2 additions & 0 deletions common/text/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ cc_library(
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:string_view",
"@com_googlesource_code_re2//:re2",
],
)

Expand All @@ -143,6 +144,7 @@ cc_test(
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:string_view",
"@com_googlesource_code_re2//:re2",
"@com_google_googletest//:gtest",
"@com_google_googletest//:gtest_main",
],
Expand Down
16 changes: 16 additions & 0 deletions common/text/config_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <initializer_list>
#include <iterator>
#include <limits>
#include <memory>
#include <string>
#include <utility>
#include <vector>
Expand All @@ -32,6 +33,7 @@
#include "absl/strings/str_split.h"
#include "absl/strings/string_view.h"
#include "common/util/logging.h"
#include "re2/re2.h"

namespace verible {
using absl::string_view;
Expand Down Expand Up @@ -184,5 +186,19 @@ ConfigValueSetter SetNamedBits(
};
}

ConfigValueSetter SetRegex(std::unique_ptr<re2::RE2> *regex) {
CHECK(regex) << "Must provide pointer to a RE2 to store.";
return [regex](string_view v) {
*regex = std::make_unique<re2::RE2>(v, re2::RE2::Quiet);
if((*regex)->ok()) {
return absl::OkStatus();
}

std::string error_msg =
absl::StrCat("Failed to parse regular expression: ", (*regex)->error());
return absl::InvalidArgumentError(error_msg);
};
}

} // namespace config
} // namespace verible
6 changes: 6 additions & 0 deletions common/text/config_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
#include <cstdint>
#include <functional>
#include <initializer_list>
#include <memory>
#include <string>
#include <vector>

#include "absl/status/status.h"
#include "absl/strings/string_view.h"
#include "re2/re2.h"

namespace verible {
namespace config {
Expand Down Expand Up @@ -94,6 +96,10 @@ ConfigValueSetter SetStringOneOf(std::string *value,
// up to 32 choices.
ConfigValueSetter SetNamedBits(uint32_t *value,
const std::vector<absl::string_view> &choices);

// Set a Regex
ConfigValueSetter SetRegex(std::unique_ptr<re2::RE2> *regex);

} // namespace config
} // namespace verible

Expand Down
30 changes: 30 additions & 0 deletions common/text/config_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@

#include "absl/status/status.h"
#include "absl/strings/match.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"
#include "gtest/gtest.h"
#include "re2/re2.h"

namespace verible {
namespace config {
Expand Down Expand Up @@ -93,6 +95,19 @@ TEST(ConfigUtilsTest, ParseBool) {
absl::StartsWith(s.message(), "baz: Boolean value should be one of"));
}

TEST(ConfigUtilsTest, ParseRegex) {
absl::Status s;
std::unique_ptr<re2::RE2> regex;
s = ParseNameValues("regex:[a-b0-9_]", {{"regex", SetRegex(&regex)}});
EXPECT_TRUE(s.ok());
EXPECT_EQ(regex->pattern(), "[a-b0-9_]");

s = ParseNameValues("regex:[a-b0-9_", {{"regex", SetRegex(&regex)}});
EXPECT_FALSE(s.ok());
EXPECT_EQ(s.message(),
"regex: Failed to parse regular expression: missing ]: [a-b0-9_");
}

TEST(ConfigUtilsTest, ParseString) {
absl::Status s;
std::string str;
Expand Down Expand Up @@ -170,6 +185,21 @@ TEST(ConfigUtilsTest, ParseMultipleParameters) {
EXPECT_TRUE(s.ok()) << s.message();
EXPECT_TRUE(panic);
EXPECT_EQ(answer, 43);

std::string str1;
std::string str2;
s = ParseNameValues("baz:hello world;fry:multiple spaces in this one",
{{"baz", SetString(&str1)}, {"fry", SetString(&str2)}});
EXPECT_TRUE(s.ok());
EXPECT_EQ(str1, "hello world");
EXPECT_EQ(str2, "multiple spaces in this one");

std::unique_ptr<re2::RE2> regex;
s = ParseNameValues("baz:some text string;regex:[A-B0-9_]",
{{"baz", SetString(&str1)}, {"regex", SetRegex(&regex)}});
EXPECT_TRUE(s.ok());
EXPECT_EQ(str1, "some text string");
EXPECT_EQ(regex->pattern(), "[A-B0-9_]");
}
} // namespace config
} // namespace verible
5 changes: 4 additions & 1 deletion verilog/analysis/checkers/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1457,10 +1457,11 @@ cc_library(
"//common/analysis:syntax-tree-lint-rule",
"//common/analysis/matcher",
"//common/analysis/matcher:bound-symbol-manager",
"//common/analysis/matcher:core-matchers",
"//common/analysis/matcher:matcher-builders",
"//common/text:concrete-syntax-tree",
"//common/text:symbol",
"//common/text:syntax-tree-context",
"//common/text:tree-utils",
"//verilog/CST:verilog-matchers",
"//verilog/CST:verilog-nonterminals",
"//verilog/analysis:descriptions",
Expand Down Expand Up @@ -1744,6 +1745,7 @@ cc_library(
"//common/analysis:syntax-tree-lint-rule",
"//common/analysis/matcher",
"//common/analysis/matcher:bound-symbol-manager",
"//common/text:config-utils",
"//common/text:symbol",
"//common/text:syntax-tree-context",
"//verilog/CST:context-functions",
Expand All @@ -1752,6 +1754,7 @@ cc_library(
"//verilog/analysis:descriptions",
"//verilog/analysis:lint-rule-registry",
"//verilog/parser:verilog-token-enum",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings:string_view",
],
alwayslink = 1,
Expand Down
3 changes: 2 additions & 1 deletion verilog/analysis/checkers/case_missing_default_rule.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@
#include "absl/strings/string_view.h"
#include "common/analysis/lint_rule_status.h"
#include "common/analysis/matcher/bound_symbol_manager.h"
#include "common/analysis/matcher/core_matchers.h"
#include "common/analysis/matcher/matcher.h"
#include "common/analysis/matcher/matcher_builders.h"
#include "common/text/concrete_syntax_tree.h"
#include "common/text/symbol.h"
#include "common/text/syntax_tree_context.h"
#include "common/text/tree_utils.h"
#include "verilog/CST/verilog_matchers.h"
#include "verilog/CST/verilog_nonterminals.h"
#include "verilog/analysis/descriptions.h"
Expand Down
Loading

0 comments on commit 82728e7

Please sign in to comment.