Merge branch 'chipsalliance:master' into instance_shadowing_fix_modport
diff --git a/.github/bin/run-clang-tidy-cached.cc b/.github/bin/run-clang-tidy-cached.cc index 8f4e7d6..d0d48e1 100755 --- a/.github/bin/run-clang-tidy-cached.cc +++ b/.github/bin/run-clang-tidy-cached.cc
@@ -15,8 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Location: https://github.com/hzeller/dev-tools -// Last update: 2024-09-16 cf86e3b6c51dbf620a08fb2a59f6ea71b6bebe3d +// Location: https://github.com/hzeller/dev-tools (2024-09-19) // Script to run clang-tidy on files in a bazel project while caching the // results as clang-tidy can be pretty slow. The clang-tidy output messages @@ -62,18 +61,19 @@ // Dont' change anything here, override for your project below in kConfig. struct ConfigValues { // Prefix for the cache to write to. If empty, use name of toplevel directory. + // Typical would be name of project with underscore suffix. std::string_view cache_prefix; // Directory to start recurse for sources createing a file list. - // Some projects need e.g. "src/". + // Some projects e.g. start at "src/". std::string_view start_dir; - // Regular expression matching files that should be included from file list. + // Regular expression matching files that should be included in file list. std::string_view file_include_re = ".*"; // Regular expxression matching files that should be excluded from file list. - // If overriding, make sure to include at least ".git". - std::string_view file_exclude_re = ".git/|.github/"; + // If searching from toplevel, make sure to include at least ".git/". + std::string_view file_exclude_re = "^(\\.git|\\.github|build)/"; // A file in the toplevel of the project that should exist, typically // something used to set up the build environment, such as MODULE.bazel, @@ -86,6 +86,11 @@ // (Default configuration: just .clang-tidy as this should always be there) std::string_view toplevel_build_file = ".clang-tidy"; + // Bazel projects have some complicated paths where generated and external + // sources reside. Setting this to true will tell this script canonicalize + // these in the output. Set to true if this is a bazel project. + bool is_bazel_project = false; + // If the compilation DB or toplevel_build_file changed in timestamp, it // might be worthwhile revisiting sources that previously had issues. // This flag enables that. @@ -94,6 +99,14 @@ // few problematic sources to begin with, otherwise every update of the // compilation DB will re-trigger revisiting all of them. bool revisit_brokenfiles_if_build_config_newer = true; + + // Revisit a source file if any of its include files changed content. Say + // foo.cc includes bar.h. Reprocess foo.cc with clang-tidy when bar.h changed, + // even if foo.cc is unchanged. This will find issues in which foo.cc relies + // on something bar.h provides. + // Usually good to keep on, but it can result in situations in which a header + // that is included by a lot of other files results in lots of reprocessing. + bool revisit_if_any_include_changes = true; }; // --------------[ Project-specific configuration ]-------------- @@ -101,15 +114,27 @@ .cache_prefix = "verible_", .file_exclude_re = ".git/|.github/" // stuff we're not interested in "|vscode/" // some generated code in there - "|tree_operations_test" // very slow - "|symbol_table_test", + "|tree_operations_test" // very slow to process. + "|symbol_table_test", // ... .toplevel_build_file = "MODULE.bazel", + .is_bazel_project = true, }; // -------------------------------------------------------------- -// Files to be considered. +// Files that look like relevant include files. +inline bool IsIncludeExtension(const std::string &extension) { + for (const std::string_view e : {".h", ".hpp", ".hxx", ".inl"}) { + if (extension == e) return true; + } + return false; +} + +// Filter for source files to be considered. inline bool ConsiderExtension(const std::string &extension) { - return extension == ".cc" || extension == ".cpp" || extension == ".h"; + for (const std::string_view e : {".cc", ".cpp", ".cxx"}) { + if (extension == e) return true; + } + return IsIncludeExtension(extension); } // Configuration of clang-tidy itself. @@ -289,6 +314,10 @@ // Use major version as part of name of our configuration specific dir. auto version = GetCommandOutput(clang_tidy_ + " --version"); + if (version.empty()) { + std::cerr << "Could not invoke " << clang_tidy_ << "; is it in PATH ?\n"; + exit(EXIT_FAILURE); + } std::smatch version_match; const std::string major_version = std::regex_search(version, version_match, std::regex{"version ([0-9]+)"}) @@ -303,16 +332,18 @@ } // Fix filename paths found in logfiles that are not emitted relative to - // project root in the log (bazel has its own, so this fixes bazel-specific - // projects) + // project root in the log - remove that prefix. + // (bazel has its own, so if this is bazel, also bazel-specific fix up that). static void RepairFilenameOccurences(const fs::path &infile, const fs::path &outfile) { static const std::regex sFixPathsRe = []() { std::string canonicalize_expr = "(^|\\n)("; // fix names at start of line - auto root = GetCommandOutput("bazel info execution_root 2>/dev/null"); - if (!root.empty()) { - root.pop_back(); // remove newline. - canonicalize_expr += root + "/|"; + if (kConfig.is_bazel_project) { + auto bzroot = GetCommandOutput("bazel info execution_root 2>/dev/null"); + if (!bzroot.empty()) { + bzroot.pop_back(); // remove newline. + canonicalize_expr += bzroot + "/|"; + } } canonicalize_expr += fs::current_path().string() + "/"; // $(pwd)/ canonicalize_expr += @@ -362,34 +393,41 @@ } // Remember content hash of header, so that we can make changed headers // influence the hash of a file including this. - if (extension == ".h") { + if (kConfig.revisit_if_any_include_changes && + IsIncludeExtension(extension)) { // Since the files might be included sloppily without prefix path, // just keep track of the basename (but since there might be collisions, // accomodate all of them by xor-ing the hashes). - const std::string just_basename = fs::path(file).filename(); + const std::string just_basename = p.filename(); header_hashes[just_basename] ^= hashContent(GetContent(p)); } } std::cerr << files_of_interest_.size() << " files of interest.\n"; - // Create content hash address. If any header a file depends on changes, we - // want to reprocess. So we make the hash dependent on header content as - // well. + // Create content hash address for the cache and build list of work items. + // If we want to revisit if headers changed, make hash dependent on them. std::list<filepath_contenthash_t> work_queue; - const std::regex inc_re("\"([0-9a-zA-Z_/-]+\\.h)\""); // match include - for (filepath_contenthash_t &f : files_of_interest_) { - const auto content = GetContent(f.first); - f.second = hashContent(content); - for (ReIt it(content.begin(), content.end(), inc_re); it != ReIt(); - ++it) { - const std::string &header_path = (*it)[1].str(); - const std::string header_basename = fs::path(header_path).filename(); - f.second ^= header_hashes[header_basename]; + const std::regex inc_re( + R"""(#\s*include\s+"([0-9a-zA-Z_/-]+\.[a-zA-Z]+)")"""); + for (filepath_contenthash_t &work_file : files_of_interest_) { + const auto content = GetContent(work_file.first); + work_file.second = hashContent(content); + if (kConfig.revisit_if_any_include_changes) { + // Update the hash with all the hashes from all include files. + for (ReIt it(content.begin(), content.end(), inc_re); it != ReIt(); + ++it) { + const std::string &header_path = (*it)[1].str(); + const std::string header_basename = fs::path(header_path).filename(); + const auto found = header_hashes.find(header_basename); + if (found != header_hashes.end()) { + work_file.second ^= found->second; + } + } } - // Recreate if we don't have it yet or if it contains messages but is - // older than WORKSPACE or compilation db. Maybe something got fixed. - if (store_.NeedsRefresh(f, min_freshness)) { - work_queue.emplace_back(f); + // Recreate if we don't have it yet or if it contains findings but is + // older than build environment. Maybe something got fixed: revisit file. + if (store_.NeedsRefresh(work_file, min_freshness)) { + work_queue.emplace_back(work_file); } } return work_queue; @@ -397,11 +435,14 @@ // Tally up findings for files of interest and assemble in one file. // (BuildWorkList() needs to be called first). - size_t CreateReport(const fs::path &project_dir, + size_t CreateReport(const fs::path &cache_dir, std::string_view symlink_detail, std::string_view symlink_summary) const { - const fs::path tidy_outfile = project_dir / "tidy.out"; - const fs::path tidy_summary = project_dir / "tidy-summary.out"; + // Make it possible to keep independent reports for different invocation + // locations (e.g. two checkouts of the same project) using the same cache. + const std::string suffix = ToHex(hashContent(fs::current_path().string())); + const fs::path tidy_outfile = cache_dir / ("tidy.out-" + suffix); + const fs::path tidy_summary = cache_dir / ("tidy-summary.out-" + suffix); // Assemble the separate outputs into a single file. Tally up per-check const std::regex check_re("(\\[[a-zA-Z.-]+\\])\n");
diff --git a/.github/workflows/verible-ci.yml b/.github/workflows/verible-ci.yml index 1a10ceb..5202674 100644 --- a/.github/workflows/verible-ci.yml +++ b/.github/workflows/verible-ci.yml
@@ -115,7 +115,7 @@ name: "diag" path: "**/plot_*.svg" - RunBantBuildCleaneer: + RunBantBuildCleaner: # Running http://bant.build/ to check all dependencies in BUILD files. runs-on: ubuntu-24.04 steps: @@ -124,28 +124,10 @@ 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.5" + VERSION="v0.1.7" 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" @@ -153,6 +135,17 @@ ln -sf ../"${STATIC_VERSION}/bin/bant" bin/ bin/bant -V + - name: Build Project genrules + run: | + # Fetch all dependencies and run genrules for bant to see every file + # that makes it into the compile. Use bant itself to find genrules. + bazel fetch ... + bazel build $(bin/bant -q genrule-outputs | awk '{print $2}') \ + //common/analysis:command-file-lexer \ + //verilog/parser:verilog-lex \ + //verilog/parser:verilog-y \ + //verilog/parser:verilog-y-final + - name: Run bant build-cleaner run: | bin/bant dwyu ...
diff --git a/verilog/analysis/checkers/BUILD b/verilog/analysis/checkers/BUILD index fdbacba..7a28339 100644 --- a/verilog/analysis/checkers/BUILD +++ b/verilog/analysis/checkers/BUILD
@@ -24,6 +24,7 @@ ":disable-statement-rule", ":endif-comment-rule", ":enum-name-style-rule", + ":explicit-begin-rule", ":explicit-function-lifetime-rule", ":explicit-function-task-parameter-type-rule", ":explicit-parameter-storage-type-rule", @@ -1254,6 +1255,40 @@ ) cc_library( + name = "explicit-begin-rule", + srcs = ["explicit_begin_rule.cc"], + hdrs = ["explicit_begin_rule.h"], + deps = [ + "//common/analysis:lint-rule-status", + "//common/analysis:token-stream-lint-rule", + "//common/text:config-utils", + "//common/text:token-info", + "//verilog/analysis:descriptions", + "//verilog/analysis:lint-rule-registry", + "//verilog/parser:verilog-token-enum", + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/status", + "@com_google_absl//absl/strings", + "@com_google_absl//absl/strings:string_view", + ], + alwayslink = 1, +) + +cc_test( + name = "explicit-begin-rule_test", + srcs = ["explicit_begin_rule_test.cc"], + deps = [ + ":explicit-begin-rule", + "//common/analysis:linter-test-utils", + "//common/analysis:token-stream-linter-test-utils", + "//verilog/analysis:verilog-analyzer", + "//verilog/parser:verilog-token-enum", + "@com_google_googletest//:gtest", + "@com_google_googletest//:gtest_main", + ], +) + +cc_library( name = "explicit-function-lifetime-rule", srcs = ["explicit_function_lifetime_rule.cc"], hdrs = ["explicit_function_lifetime_rule.h"],
diff --git a/verilog/analysis/checkers/explicit_begin_rule.cc b/verilog/analysis/checkers/explicit_begin_rule.cc new file mode 100644 index 0000000..44fee05 --- /dev/null +++ b/verilog/analysis/checkers/explicit_begin_rule.cc
@@ -0,0 +1,373 @@ +// Copyright 2017-2020 The Verible Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "verilog/analysis/checkers/explicit_begin_rule.h" + +#include <set> + +#include "absl/base/attributes.h" +#include "absl/status/status.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" +#include "common/analysis/lint_rule_status.h" +#include "common/analysis/token_stream_lint_rule.h" +#include "common/text/config_utils.h" +#include "common/text/token_info.h" +#include "verilog/analysis/descriptions.h" +#include "verilog/analysis/lint_rule_registry.h" +#include "verilog/parser/verilog_token_enum.h" + +namespace verilog { +namespace analysis { + +using verible::LintRuleStatus; +using verible::LintViolation; +using verible::TokenInfo; +using verible::TokenStreamLintRule; + +// Register the lint rule +VERILOG_REGISTER_LINT_RULE(ExplicitBeginRule); + +static const char kMessage[] = + " block constructs shall explicitly use begin/end."; + +const LintRuleDescriptor &ExplicitBeginRule::GetDescriptor() { + static const LintRuleDescriptor d{ + .name = "explicit-begin", + .topic = "explicit-begin", + .desc = + "Checks that a Verilog ``begin`` directive follows all " + "if, else, always, always_comb, always_latch, always_ff, " + "for, forever, foreach, while and initial statements.", + .param = + { + {"if_enable", "true", + "All if statements require an explicit begin-end block"}, + {"else_enable", "true", + "All else statements require an explicit begin-end block"}, + {"always_enable", "true", + "All always statements require an explicit begin-end block"}, + {"always_comb_enable", "true", + "All always_comb statements require an explicit begin-end " + "block"}, + {"always_latch_enable", "true", + "All always_latch statements require an explicit begin-end " + "block"}, + {"always_ff_enable", "true", + "All always_ff statements require an explicit begin-end block"}, + {"for_enable", "true", + "All for statements require an explicit begin-end block"}, + {"forever_enable", "true", + "All forever statements require an explicit begin-end block"}, + {"foreach_enable", "true", + "All foreach statements require an explicit begin-end block"}, + {"while_enable", "true", + "All while statements require an explicit begin-end block"}, + {"initial_enable", "true", + "All initial statements require an explicit begin-end block"}, + }, + }; + return d; +} + +absl::Status ExplicitBeginRule::Configure(absl::string_view configuration) { + using verible::config::SetBool; + return verible::ParseNameValues( + configuration, + { + {"if_enable", SetBool(&if_enable_)}, + {"else_enable", SetBool(&else_enable_)}, + {"always_enable", SetBool(&always_enable_)}, + {"always_comb_enable", SetBool(&always_comb_enable_)}, + {"always_latch_enable", SetBool(&always_latch_enable_)}, + {"always_ff_enable", SetBool(&always_ff_enable_)}, + {"for_enable", SetBool(&for_enable_)}, + {"forever_enable", SetBool(&forever_enable_)}, + {"foreach_enable", SetBool(&foreach_enable_)}, + {"while_enable", SetBool(&while_enable_)}, + {"initial_enable", SetBool(&initial_enable_)}, + }); +} + +bool ExplicitBeginRule::IsTokenEnabled(const TokenInfo &token) { + switch (token.token_enum()) { + case TK_if: + return if_enable_; + case TK_else: + return else_enable_; + case TK_always: + return always_enable_; + case TK_always_comb: + return always_comb_enable_; + case TK_always_latch: + return always_latch_enable_; + case TK_always_ff: + return always_ff_enable_; + case TK_for: + return for_enable_; + case TK_forever: + return forever_enable_; + case TK_foreach: + return foreach_enable_; + case TK_while: + return while_enable_; + case TK_initial: + return initial_enable_; + default: + return false; + } +} + +bool ExplicitBeginRule::HandleTokenStateMachine(const TokenInfo &token) { + // Responds to a token by updating the state of the analysis. + bool raise_violation = false; + switch (state_) { + case State::kNormal: { + // Special handling for constraints + switch (token.token_enum()) { + case TK_constraint: { + constraint_expr_level_ = 0; + state_ = State::kConstraint; + return false; + } + case TK_with: { + constraint_expr_level_ = 0; + state_ = State::kInlineConstraint; + return false; + } + default: { + // Do nothing + break; + } + } + + if (!IsTokenEnabled(token)) { + break; + } + + switch (token.token_enum()) { + // After token expect "begin" + case TK_always_comb: + ABSL_FALLTHROUGH_INTENDED; + case TK_always_latch: + ABSL_FALLTHROUGH_INTENDED; + case TK_forever: + ABSL_FALLTHROUGH_INTENDED; + case TK_initial: + start_token_ = token; + state_ = State::kExpectBegin; + break; + // After token expect a "condition" followed by "begin". NOTE: there may + // be tokens prior to the condition (like in an "always_ff" statement) + // and these are all ignored. + case TK_if: + ABSL_FALLTHROUGH_INTENDED; + case TK_always_ff: + ABSL_FALLTHROUGH_INTENDED; + case TK_for: + ABSL_FALLTHROUGH_INTENDED; + case TK_foreach: + ABSL_FALLTHROUGH_INTENDED; + case TK_while: + condition_expr_level_ = 0; + start_token_ = token; + state_ = State::kInCondition; + break; + // always gets special handling, as somtimes there is a "condition" or + // not before a "begin". + case TK_always: + condition_expr_level_ = 0; + start_token_ = token; + state_ = State::kInAlways; + break; + // else is also special as "if" or "begin" can follow + case TK_else: + start_token_ = token; + state_ = State::kInElse; + break; + default: + // Do nothing + break; + } + break; + } + case State::kInAlways: { + // always is a little more complicated in that it can be immediately + // followed by a "begin" or followed by some special characters ("@" or + // "*") and maybe a condition. + switch (token.token_enum()) { + case '@': + break; + case '*': + break; + case TK_begin: + state_ = State::kNormal; + break; + case '(': + condition_expr_level_++; + state_ = State::kInCondition; + break; + default: + raise_violation = true; + break; + } + break; + } + case State::kInElse: { + // An else statement can be followed by either a begin or an if. + switch (token.token_enum()) { + case TK_if: + if (if_enable_) { + condition_expr_level_ = 0; + start_token_ = token; + state_ = State::kInCondition; + } else { + state_ = State::kNormal; + } + break; + case TK_begin: + state_ = State::kNormal; + break; + default: + raise_violation = true; + break; + } + break; + } + case State::kInCondition: { + // The last token expects a condition statement enclosed in a pair of + // parentheses "()". This process also ignores any tokens between the last + // token and the opening parentheses which simplifies "always_ff". + switch (token.token_enum()) { + case '(': { + condition_expr_level_++; + break; + } + case ')': { + condition_expr_level_--; + if (condition_expr_level_ == 0) { + state_ = State::kExpectBegin; + } + break; + } + default: { + // throw away everything else + break; + } + } + break; + } + case State::kExpectBegin: { + // The next token must be a "begin" + switch (token.token_enum()) { + case TK_begin: + // If we got our begin token, we go back to normal status + state_ = State::kNormal; + break; + default: { + raise_violation = true; + break; + } + } + break; + } + case State::kInlineConstraint: { + switch (token.token_enum()) { + case '{': { + // An InlineConstraint + constraint_expr_level_++; + state_ = State::kConstraint; + break; + } + default: { + // throw away everything else + state_ = State::kNormal; + break; + } + } + } + ABSL_FALLTHROUGH_INTENDED; + case State::kConstraint: { + // System Verilog constraints are special and use curly braces {} + // instead of begin-end. So ignore all constraints + switch (token.token_enum()) { + case '{': { + constraint_expr_level_++; + break; + } + case '}': { + constraint_expr_level_--; + if (constraint_expr_level_ == 0) { + state_ = State::kNormal; + } + break; + } + default: { + // throw away everything else + break; + } + } + break; + } + default: { + // Do nothing + break; + } + } // switch (state_) + + if (raise_violation) { + violations_.insert(LintViolation( + start_token_, absl::StrCat(start_token_.text(), kMessage, + " Expected begin, got ", token.text()))); + + // Once the violation is raised, we go back to a normal, default, state + state_ = State::kNormal; + } + + return raise_violation; +} + +void ExplicitBeginRule::HandleToken(const TokenInfo &token) { + // Ignore all white space and comments and return immediately + switch (token.token_enum()) { + case TK_SPACE: + return; + case TK_NEWLINE: + return; + case TK_COMMENT_BLOCK: + return; + case TK_EOL_COMMENT: + return; + default: + break; + } + + bool retry = HandleTokenStateMachine(token); + + // If this token raises a violation, it was because the statemachine was + // expecting a begin token. This token may expect a begin token too, so + // check it. + // Consider: forever if(a) #10; else #20; // 3 violations could be raised + // [forever, if, else]. + if (retry) { + HandleTokenStateMachine(token); + } +} + +LintRuleStatus ExplicitBeginRule::Report() const { + return LintRuleStatus(violations_, GetDescriptor()); +} + +} // namespace analysis +} // namespace verilog \ No newline at end of file
diff --git a/verilog/analysis/checkers/explicit_begin_rule.h b/verilog/analysis/checkers/explicit_begin_rule.h new file mode 100644 index 0000000..a91205a --- /dev/null +++ b/verilog/analysis/checkers/explicit_begin_rule.h
@@ -0,0 +1,94 @@ +// Copyright 2017-2020 The Verible Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef VERIBLE_VERILOG_ANALYSIS_CHECKERS_EXPLICIT_BEGIN_RULE_H_ +#define VERIBLE_VERILOG_ANALYSIS_CHECKERS_EXPLICIT_BEGIN_RULE_H_ + +#include <set> + +#include "absl/status/status.h" +#include "absl/strings/string_view.h" +#include "common/analysis/lint_rule_status.h" +#include "common/analysis/token_stream_lint_rule.h" +#include "common/text/token_info.h" +#include "verilog/analysis/descriptions.h" + +namespace verilog { +namespace analysis { + +using verible::TokenInfo; + +// Detects whether if and for loop statements use verilog block statements +// (begin/end) +class ExplicitBeginRule : public verible::TokenStreamLintRule { + public: + using rule_type = verible::TokenStreamLintRule; + + static const LintRuleDescriptor &GetDescriptor(); + + absl::Status Configure(absl::string_view configuration) final; + + void HandleToken(const verible::TokenInfo &token) final; + + verible::LintRuleStatus Report() const final; + + private: + bool HandleTokenStateMachine(const TokenInfo &token); + + bool IsTokenEnabled(const TokenInfo &token); + + // States of the internal token-based analysis. + enum class State { + kNormal, + kInAlways, + kInCondition, + kInElse, + kExpectBegin, + kConstraint, + kInlineConstraint + }; + + // Internal lexical analysis state. + State state_{State::kNormal}; + + // Level of nested parenthesis when analysing conditional expressions + int condition_expr_level_{0}; + + // Level inside a constraint expression + int constraint_expr_level_{0}; + + // Configuration + bool if_enable_{true}; + bool else_enable_{true}; + bool always_enable_{true}; + bool always_comb_enable_{true}; + bool always_latch_enable_{true}; + bool always_ff_enable_{true}; + bool for_enable_{true}; + bool forever_enable_{true}; + bool foreach_enable_{true}; + bool while_enable_{true}; + bool initial_enable_{true}; + + // Token that requires blocking statement. + verible::TokenInfo start_token_{verible::TokenInfo::EOFToken()}; + + // Collection of found violations. + std::set<verible::LintViolation> violations_; +}; + +} // namespace analysis +} // namespace verilog + +#endif // VERIBLE_VERILOG_ANALYSIS_CHECKERS_ENDIF_COMMENT_RULE_H_ \ No newline at end of file
diff --git a/verilog/analysis/checkers/explicit_begin_rule_test.cc b/verilog/analysis/checkers/explicit_begin_rule_test.cc new file mode 100644 index 0000000..50d0d66 --- /dev/null +++ b/verilog/analysis/checkers/explicit_begin_rule_test.cc
@@ -0,0 +1,492 @@ +// Copyright 2017-2020 The Verible Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "verilog/analysis/checkers/explicit_begin_rule.h" + +#include <initializer_list> + +#include "common/analysis/linter_test_utils.h" +#include "common/analysis/token_stream_linter_test_utils.h" +#include "gtest/gtest.h" +#include "verilog/analysis/verilog_analyzer.h" +#include "verilog/parser/verilog_token_enum.h" + +namespace verilog { +namespace analysis { +namespace { + +using verible::LintTestCase; +using verible::RunConfiguredLintTestCases; +using verible::RunLintTestCases; + +// Tests that space-only text passes. +TEST(ExplicitBeginRuleTest, AcceptsBlank) { + const std::initializer_list<LintTestCase> kTestCases = { + {""}, + {" "}, + {"\n"}, + {" \n\n"}, + }; + RunLintTestCases<VerilogAnalyzer, ExplicitBeginRule>(kTestCases); +} + +// Tests that properly matched if/begin passes. +TEST(ExplicitBeginRuleTest, AcceptsBlocksWithBegin) { + const std::initializer_list<LintTestCase> kTestCases = { + {"if (FOO) /*block comment */ begin a <= 1;"}, + {"if (FOO) begin a <= 1;"}, + {"if (FOO)begin : name_statement a <= 1;"}, + {"if (FOO)\n begin a <= 1;"}, + {"if (FOO) //Comment\n begin a <= 1;"}, + + {"else begin \n FOO"}, + {"else \nbegin \n FOO"}, + {"else //Comment\n begin \n FOO"}, + {"else \n //Comment\n begin \n FOO"}, + {"else if (FOO) begin a <= 1;"}, + {"else if (FOO)\n begin a <= 1;"}, + {"else if (FOO) //Comment\n begin a <= 1;"}, + {"else if (FOO)\n //Comment\n begin a <= 1;"}, + + {"for(i = 0; i < N; i++) begin a <= 1;"}, + {"for(i = 0; i < N; i++)\nbegin a <= 1;"}, + {"for(i = 0; i < N; i++) // Comment\n begin a <= 1;"}, + {"for(i = 0; i < N; i++)\n // Comment\nbegin a <= 1;"}, + + {"foreach(array[i]) begin a <= 1;"}, + {"foreach(array[i])\nbegin a <= 1;"}, + {"foreach(array[i]) // Comment\n begin a <= 1;"}, + {"foreach(array[i])\n // Comment\nbegin a <= 1;"}, + + {"while (a < 3) begin a = a + 1;"}, + {"while(a < 3)\nbegin a = a + 1;"}, + {"while (a < 3) // Comment\n begin a = a + 1;"}, + {"while(a < 3)\n // Comment\nbegin a = a + 1;"}, + + {"forever begin a <= 1;"}, + {"forever\nbegin a <= 1;"}, + {"forever // Comment\n begin a <= 1;"}, + {"forever\n // Comment\nbegin a <= 1;"}, + + {"initial begin a <= 1;"}, + {"initial\nbegin a <= 1;"}, + {"initial // Comment\n begin a <= 1;"}, + {"initial\n // Comment\nbegin a <= 1;"}, + + {"always_comb begin a = 1;"}, + {"always_comb\nbegin a = 1;"}, + {"always_comb // Comment\n begin a = 1;"}, + {"always_comb\n // Comment\nbegin a = 1;"}, + + {"always_latch begin a <= 1;"}, + {"always_latch\nbegin a <= 1;"}, + {"always_latch // Comment\n begin a <= 1;"}, + {"always_latch\n // Comment\nbegin a <= 1;"}, + + {"always_ff @( a or b) begin a <= 1;"}, + {"always_ff @ ( a or b)\nbegin a <= 1;"}, + {"always_ff @( (a) and b) // Comment\n begin a <= 1;"}, + {"always_ff @( a or ((b)))\n // Comment\nbegin a <= 1;"}, + + {"always @( a or b) begin a <= 1;"}, + {"always @ ( a or b)\nbegin a <= 1;"}, + {"always @( (a) and b) // Comment\n begin a <= 1;"}, + {"always @( a or ((b)))\n // Comment\nbegin a <= 1;"}, + {"always@* begin a = 1'b1;"}, + {"always@(*) begin a = 1'b1;"}, + {"always @* begin a = 1'b1;"}, + {"always begin a = 1'b1;"}, + {"always begin #10 a = 1'b1;"}, + + // Ignore constraints + {"constraint c_array { foreach (array[i]) {array[i] == i;}}"}, + {"constraint c {if(a == 2){b == 1;}else{b == 2;}}"}, + + // Ignore inline constraints + {"task a(); std::randomize(b) with {foreach(b[i]){b[i] inside " + "{[0:1024]};}}; endtask"}, + {"task a(); std::randomize(b) with {if(a == 2){b == 1;}else{b == 2;}}; " + "endtask"}, + + // Multiple consecutive failures + {"if(FOO) begin for(i = 0; i < N; i++) begin a <= i;"}, + {"if(FOO) begin foreach(array[i]) begin a <= i;"}, + {"if(FOO) begin while(i < N) begin i++;"}, + {"for(i = 0; i < N; i++) begin if (FOO) begin a <= 1'b1;"}, + {"always @* begin if(FOO) begin a = 1; end else begin a = 0;"}, + {"always @(*) begin if(FOO) begin a = 1; end else begin a = 0;"}, + {"always @(posedge c) begin if(FOO) begin a <= 1; end else begin " + "a <= 0;"}, + {"always_comb begin if(FOO) begin a = 1; end else begin a = 0;"}, + {"always_ff @(posedge c) begin if(FOO) begin a <= 1; end else begin " + "a <= 0;"}, + {"constraint c_array { foreach (array[i]) {array[i] == i;}}if(FOO) begin " + "a <= 1;end"}, + {"if(FOO) begin a <= 1;end constraint c {if(a == 2){b == 1;}else{b == " + "2;}}"}, + }; + + RunLintTestCases<VerilogAnalyzer, ExplicitBeginRule>(kTestCases); +} + +// Tests that unmatched block/begin fails is detected. +TEST(ExplicitBeginRuleTest, RejectBlocksWithoutBegin) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {{TK_if, "if"}, " (FOO)\n BAR"}, + {{TK_if, "if"}, " (FOO) //Comment\n BAR"}, + + {{TK_else, "else"}, " \n FOO"}, + {{TK_else, "else"}, " \n \n FOO"}, + {{TK_else, "else"}, " //Comment\n FOO"}, + {{TK_else, "else"}, " \n //Comment\n FOO"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO)\n BAR"}, + {"else ", {TK_if, "if"}, " (FOO) //Comment\n BAR"}, + {"else ", {TK_if, "if"}, " (FOO)\n //Comment\n BAR"}, + + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_for, "for"}, "(i = 0; i < N; i++)\n a <= 1'b1;"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) // Comment \n a <= 1'b1;"}, + {{TK_for, "for"}, "(i = 0; i < N; i++)\n // Comment\n a <= 1'b1;"}, + + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i])\n a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) // Comment \n a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i])\n // Comment\n a <= 1'b1;"}, + + {{TK_while, "while"}, "(i < N) a <= 1'b1;"}, + {{TK_while, "while"}, " (i < N)\n a <= 1'b1;"}, + {{TK_while, "while"}, "(i < N) // Comment \n a <= 1'b1;"}, + {{TK_while, "while"}, " (i < N)\n // Comment\n a <= 1'b1;"}, + + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_forever, "forever"}, "\n a <= 1'b1;"}, + {{TK_forever, "forever"}, " // Comment \n a <= 1'b1;"}, + {{TK_forever, "forever"}, "\n // Comment\n a <= 1'b1;"}, + + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_initial, "initial"}, "\n a = 1'b1;"}, + {{TK_initial, "initial"}, " // Comment \n a = 1'b1;"}, + {{TK_initial, "initial"}, "\n // Comment\n a = 1'b1;"}, + + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, "\n a = 1'b1;"}, + {{TK_always_comb, "always_comb"}, " // Comment \n a = 1'b1;"}, + {{TK_always_comb, "always_comb"}, "\n // Comment\n a = 1'b1;"}, + + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, "\n a = 1'b1;"}, + {{TK_always_latch, "always_latch"}, " // Comment \n a = 1'b1;"}, + {{TK_always_latch, "always_latch"}, "\n // Comment\n a = 1'b1;"}, + + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, "@(a or b)\n a <= 1'b1;"}, + {{TK_always_ff, "always_ff"}, + " @(posedge a or negedge b) // Comment \n a <= 1'b1;"}, + {{TK_always_ff, "always_ff"}, "@(a || b)\n // Comment\n a <= 1'b1;"}, + + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + {{TK_always, "always"}, "@(a or b)\n a = 1'b1;"}, + {{TK_always, "always"}, + " @(posedge a or negedge b) // Comment \n a <= 1'b1;"}, + {{TK_always, "always"}, "@(a || b)\n // Comment\n <= 1'b1;"}, + {{TK_always, "always"}, "@* a = 1'b1;"}, + {{TK_always, "always"}, "@(*) a = 1'b1;"}, + {{TK_always, "always"}, " @* a = 1'b1;"}, + {{TK_always, "always"}, " a = 1'b1;"}, + {{TK_always, "always"}, " #10 a = 1'b1;"}, + + // Multiple consecutive failures + {{TK_if, "if"}, "(FOO) ", {TK_for, "for"}, "(i = 0; i < N; i++) a <= i;"}, + {{TK_if, "if"}, "(FOO) ", {TK_foreach, "foreach"}, "(array[i]) a <= i;"}, + {{TK_if, "if"}, "(FOO) ", {TK_while, "while"}, "(i < N) i++;"}, + {{TK_for, "for"}, + "(i = 0; i < N; i++)\n", + {TK_if, "if"}, + " (FOO) a <= 1'b1;"}, + {{TK_always, "always"}, + " @* ", + {TK_if, "if"}, + "(FOO) a = 1;", + {TK_else, "else"}, + " a = 0;"}, + {{TK_always, "always"}, + " @(*) ", + {TK_if, "if"}, + "(FOO) a = 1;", + {TK_else, "else"}, + " a = 0;"}, + {{TK_always, "always"}, + " @(posedge c) ", + {TK_if, "if"}, + "(FOO) a <= 1;", + {TK_else, "else"}, + " a <= 0;"}, + {{TK_always_comb, "always_comb"}, + " ", + {TK_if, "if"}, + "(FOO) a = 1;", + {TK_else, "else"}, + " a = 0;"}, + {{TK_always_ff, "always_ff"}, + " @(posedge c) ", + {TK_if, "if"}, + "(FOO) a <= 1;", + {TK_else, "else"}, + " a <= 0;"}, + {"constraint c_array { foreach (array[i]) array[i] == i;}", + {TK_if, "if"}, + "(FOO) a <= 1;"}, + {{TK_if, "if"}, + "(FOO) a <= 1; constraint c {if(a == 2) b == 1;else b == 2;}"}, + }; + + RunLintTestCases<VerilogAnalyzer, ExplicitBeginRule>(kTestCases); +} + +// Tests that rule can be disabled for if statements +TEST(ExplicitBeginRuleTest, AcceptsIfBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {"if (FOO) BAR"}, + {"else if (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "if_enable:false"); +} + +// Tests that rule can be disabled for else statements +TEST(ExplicitBeginRuleTest, AcceptsElseBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {"else \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "else_enable:false"); +} + +// Tests that rule can be disabled for for statements +TEST(ExplicitBeginRuleTest, AcceptsForBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {"for(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "for_enable:false"); +} + +// Tests that rule can be disabled for foreach statements +TEST(ExplicitBeginRuleTest, AcceptsForeachBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {"foreach(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "foreach_enable:false"); +} + +// Tests that rule can be disabled for while statements +TEST(ExplicitBeginRuleTest, AcceptsWhileBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {"while(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "while_enable:false"); +} + +// Tests that rule can be disabled for forever statements +TEST(ExplicitBeginRuleTest, AcceptsForeverBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {"forever a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "forever_enable:false"); +} + +// Tests that rule can be disabled for initial statements +TEST(ExplicitBeginRuleTest, AcceptsInitialBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {"initial a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "initial_enable:false"); +} + +// Tests that rule can be disabled for always_comb statements +TEST(ExplicitBeginRuleTest, AcceptsAlwaysCombBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {"always_comb a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "always_comb_enable:false"); +} + +// Tests that rule can be disabled for always_latch statements +TEST(ExplicitBeginRuleTest, AcceptsAlwaysLatchBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {"always_latch a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "always_latch_enable:false"); +} + +// Tests that rule can be disabled for always_ff statements +TEST(ExplicitBeginRuleTest, AcceptsAlwaysFFBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {"always_ff @(a or b) a <= 1'b1;\n"}, + {{TK_always, "always"}, " @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "always_ff_enable:false"); +} + +// Tests that rule can be disabled for always statements +TEST(ExplicitBeginRuleTest, AcceptsAlwaysBlocksWithoutBeginConfigured) { + const std::initializer_list<LintTestCase> kTestCases = { + {{TK_if, "if"}, " (FOO) BAR"}, + {"else ", {TK_if, "if"}, " (FOO) BAR"}, + {{TK_else, "else"}, " \n FOO"}, + {{TK_for, "for"}, "(i = 0; i < N; i++) a <= 1'b1;"}, + {{TK_foreach, "foreach"}, "(array[i]) a <= 1'b1;"}, + {{TK_while, "while"}, "(array[i]) a <= 1'b1;"}, + {{TK_forever, "forever"}, " a <= 1'b1;\n"}, + {{TK_initial, "initial"}, " a = 1'b1;\n"}, + {{TK_always_comb, "always_comb"}, " a = 1'b1;\n"}, + {{TK_always_latch, "always_latch"}, " a = 1'b1;\n"}, + {{TK_always_ff, "always_ff"}, " @(a or b) a <= 1'b1;\n"}, + {"always @(a or b) a = 1'b1;\n"}, + }; + + RunConfiguredLintTestCases<VerilogAnalyzer, ExplicitBeginRule>( + kTestCases, "always_enable:false"); +} + +} // namespace +} // namespace analysis +} // namespace verilog \ No newline at end of file
diff --git a/verilog/analysis/checkers/instance_shadow_rule.cc b/verilog/analysis/checkers/instance_shadow_rule.cc index 57d7f7d..a2edd8b 100644 --- a/verilog/analysis/checkers/instance_shadow_rule.cc +++ b/verilog/analysis/checkers/instance_shadow_rule.cc
@@ -65,6 +65,7 @@ static const Matcher matcher(SymbolIdentifierLeaf()); return matcher; } + static bool isInAllowedNode(const SyntaxTreeContext &ctx) { return ctx.IsInside(NodeEnum::kSeqBlock) || ctx.IsInside(NodeEnum::kGenvarDeclaration) ||
diff --git a/verilog/tools/lint/BUILD b/verilog/tools/lint/BUILD index 3d8d5da..1e9e22e 100644 --- a/verilog/tools/lint/BUILD +++ b/verilog/tools/lint/BUILD
@@ -61,6 +61,7 @@ ("disable-statement", "disable_statement", False), ("endif-comment", "endif_comment", False), ("enum-name-style", "enum_name_style", True), + ("explicit-begin", "explicit_begin", False), ("explicit-function-lifetime", "explicit_function_lifetime", True), ("explicit-function-task-parameter-type", "explicit_function_parameter_type", True), ("explicit-function-task-parameter-type", "explicit_task_parameter_type", True),
diff --git a/verilog/tools/lint/testdata/always_comb_blocking.sv b/verilog/tools/lint/testdata/always_comb_blocking.sv index 403cf28..e54eb43 100644 --- a/verilog/tools/lint/testdata/always_comb_blocking.sv +++ b/verilog/tools/lint/testdata/always_comb_blocking.sv
@@ -1,4 +1,5 @@ module always_comb_blocking; - always_comb + always_comb begin a <= b; // [Style: combinational-logic] [always-comb-blocking] + end endmodule
diff --git a/verilog/tools/lint/testdata/always_ff_non_blocking.sv b/verilog/tools/lint/testdata/always_ff_non_blocking.sv index a699fb8..0a10182 100644 --- a/verilog/tools/lint/testdata/always_ff_non_blocking.sv +++ b/verilog/tools/lint/testdata/always_ff_non_blocking.sv
@@ -1,4 +1,5 @@ module always_ff_non_blocking; - always_ff @(posedge c) + always_ff @(posedge c) begin a = b; // [Style: sequential-logic] [always-ff-non-blocking] + end endmodule
diff --git a/verilog/tools/lint/testdata/explicit_begin.sv b/verilog/tools/lint/testdata/explicit_begin.sv new file mode 100644 index 0000000..3aadad4 --- /dev/null +++ b/verilog/tools/lint/testdata/explicit_begin.sv
@@ -0,0 +1,4 @@ +module explicit_begin (); + always_comb + a = 1; +endmodule
diff --git a/verilog/tools/lint/testdata/generate_begin_module.sv b/verilog/tools/lint/testdata/generate_begin_module.sv index 9cd9848..5ed49e1 100644 --- a/verilog/tools/lint/testdata/generate_begin_module.sv +++ b/verilog/tools/lint/testdata/generate_begin_module.sv
@@ -2,7 +2,9 @@ // verilog_lint: waive legacy-generate-region generate begin : gen_block1 - always @(posedge clk) foo <= bar; + always @(posedge clk) begin + foo <= bar; + end end endgenerate endmodule
diff --git a/verilog/tools/lint/testdata/plusarg_assignment.sv b/verilog/tools/lint/testdata/plusarg_assignment.sv index 89f9f24..af2210f 100644 --- a/verilog/tools/lint/testdata/plusarg_assignment.sv +++ b/verilog/tools/lint/testdata/plusarg_assignment.sv
@@ -1,6 +1,8 @@ class foo; function bar; // The use of $test$plusargs() is not allowed. - if ($test$plusargs("baz")) return 1; + if ($test$plusargs("baz")) begin + return 1; + end endfunction endclass
diff --git a/verilog/tools/lint/testdata/suspicious_semicolon.sv b/verilog/tools/lint/testdata/suspicious_semicolon.sv index ab49fb6..45697b1 100644 --- a/verilog/tools/lint/testdata/suspicious_semicolon.sv +++ b/verilog/tools/lint/testdata/suspicious_semicolon.sv
@@ -1,5 +1,6 @@ module suspicious_semicolon (); initial begin + // verilog_lint: waive explicit-begin if (x); $display("Hi"); end