diff --git a/verilog/analysis/checkers/BUILD b/verilog/analysis/checkers/BUILD index fdbacba89..7a28339e6 100644 --- a/verilog/analysis/checkers/BUILD +++ b/verilog/analysis/checkers/BUILD @@ -24,6 +24,7 @@ cc_library( ":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", @@ -1253,6 +1254,40 @@ cc_test( ], ) +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"], diff --git a/verilog/analysis/checkers/explicit_begin_rule.cc b/verilog/analysis/checkers/explicit_begin_rule.cc new file mode 100644 index 000000000..44fee0531 --- /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 + +#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 000000000..a91205a45 --- /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 + +#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 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 000000000..50d0d6683 --- /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 + +#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 kTestCases = { + {""}, + {" "}, + {"\n"}, + {" \n\n"}, + }; + RunLintTestCases(kTestCases); +} + +// Tests that properly matched if/begin passes. +TEST(ExplicitBeginRuleTest, AcceptsBlocksWithBegin) { + const std::initializer_list 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(kTestCases); +} + +// Tests that unmatched block/begin fails is detected. +TEST(ExplicitBeginRuleTest, RejectBlocksWithoutBegin) { + const std::initializer_list 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(kTestCases); +} + +// Tests that rule can be disabled for if statements +TEST(ExplicitBeginRuleTest, AcceptsIfBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "if_enable:false"); +} + +// Tests that rule can be disabled for else statements +TEST(ExplicitBeginRuleTest, AcceptsElseBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "else_enable:false"); +} + +// Tests that rule can be disabled for for statements +TEST(ExplicitBeginRuleTest, AcceptsForBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "for_enable:false"); +} + +// Tests that rule can be disabled for foreach statements +TEST(ExplicitBeginRuleTest, AcceptsForeachBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "foreach_enable:false"); +} + +// Tests that rule can be disabled for while statements +TEST(ExplicitBeginRuleTest, AcceptsWhileBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "while_enable:false"); +} + +// Tests that rule can be disabled for forever statements +TEST(ExplicitBeginRuleTest, AcceptsForeverBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "forever_enable:false"); +} + +// Tests that rule can be disabled for initial statements +TEST(ExplicitBeginRuleTest, AcceptsInitialBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "initial_enable:false"); +} + +// Tests that rule can be disabled for always_comb statements +TEST(ExplicitBeginRuleTest, AcceptsAlwaysCombBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "always_comb_enable:false"); +} + +// Tests that rule can be disabled for always_latch statements +TEST(ExplicitBeginRuleTest, AcceptsAlwaysLatchBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "always_latch_enable:false"); +} + +// Tests that rule can be disabled for always_ff statements +TEST(ExplicitBeginRuleTest, AcceptsAlwaysFFBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "always_ff_enable:false"); +} + +// Tests that rule can be disabled for always statements +TEST(ExplicitBeginRuleTest, AcceptsAlwaysBlocksWithoutBeginConfigured) { + const std::initializer_list 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( + kTestCases, "always_enable:false"); +} + +} // namespace +} // namespace analysis +} // namespace verilog \ No newline at end of file diff --git a/verilog/tools/lint/BUILD b/verilog/tools/lint/BUILD index 3d8d5dace..1e9e22e8e 100644 --- a/verilog/tools/lint/BUILD +++ b/verilog/tools/lint/BUILD @@ -61,6 +61,7 @@ _linter_test_configs = [ ("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 403cf28fb..e54eb4376 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 a699fb8ba..0a10182b5 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 000000000..3aadad445 --- /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 9cd984845..5ed49e144 100644 --- a/verilog/tools/lint/testdata/generate_begin_module.sv +++ b/verilog/tools/lint/testdata/generate_begin_module.sv @@ -2,7 +2,9 @@ module generate_begin_module; // 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 89f9f2435..af2210f93 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 ab49fb6cc..45697b1ff 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