Skip to content

Commit 8ce6200

Browse files
author
Lukasz Dalek
committed
forbid-implicit-declarations: Detect generate scopes. Add more tests
Signed-off-by: Lukasz Dalek <[email protected]>
1 parent 27f8814 commit 8ce6200

File tree

3 files changed

+162
-18
lines changed

3 files changed

+162
-18
lines changed

‎verilog/analysis/checkers/forbid_implicit_declarations_rule.cc

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,10 @@ void ForbidImplicitDeclarationsRule::DetectDeclarations(const verible::Symbol& s
8787
// Top of the matched tree stack
8888
CHECK_GE(context.size(), 1);
8989
const auto& top = context.top(); // scope
90-
CHECK_EQ(static_cast<verilog::NodeEnum>(top.Tag().tag),
91-
verilog::NodeEnum::kModuleItemList);
90+
const auto top_tag = static_cast<verilog::NodeEnum>(top.Tag().tag);
91+
92+
CHECK((top_tag == NodeEnum::kModuleItemList) ||
93+
(top_tag == NodeEnum::kGenerateItemList));
9294

9395
// This could be omitted if we had matchers for begining and ending of scopes
9496
// e.g. something like kBegin/kEnd. But we would have to modify parser grammar for
@@ -114,18 +116,12 @@ void ForbidImplicitDeclarationsRule::DetectDeclarations(const verible::Symbol& s
114116
}
115117
}
116118

117-
void ForbidImplicitDeclarationsRule::DetectReference(const verible::Symbol& symbol,
118-
const SyntaxTreeContext& context) {
119-
verible::matcher::BoundSymbolManager manager;
120-
121-
if (net_assignment_matcher_.Matches(symbol, &manager)) {
122-
const auto* lval = ABSL_DIE_IF_NULL(manager.GetAs<verible::SyntaxTreeLeaf>("lval"));
123-
const verible::TokenInfo& lval_token = lval->get();
119+
void ForbidImplicitDeclarationsRule::AnalyzeLHS(const verible::SyntaxTreeLeaf& lval,
120+
const verible::SyntaxTreeContext& context) {
121+
const verible::TokenInfo& lval_token = lval.get();
124122
CHECK_EQ(lval_token.token_enum, SymbolIdentifier);
125123
const auto& identifier = lval_token.text;
126124

127-
CheckAndPopScope(symbol, context);
128-
129125
CHECK_GE(scopes.size(), 1);
130126
auto& scope = scopes.top().declared_nets_;
131127
const auto* node = scope[identifier];
@@ -135,8 +131,46 @@ void ForbidImplicitDeclarationsRule::DetectReference(const verible::Symbol& symb
135131
// TODO: Check necessity of call to ContainsAncestor(). CheckAndPopScope()
136132
// is propably enough.
137133
if ((node == nullptr) || (!ContainsAncestor(node, context))) {
138-
violations_.insert(LintViolation(*lval, kMessage, context));
134+
violations_.insert(LintViolation(lval, kMessage, context));
139135
}
136+
}
137+
138+
void ForbidImplicitDeclarationsRule::DetectReference(const verible::Symbol& symbol,
139+
const SyntaxTreeContext& context) {
140+
verible::matcher::BoundSymbolManager manager;
141+
142+
if (net_assignment_matcher_.Matches(symbol, &manager)) {
143+
if (!context.IsInside(NodeEnum::kContinuousAssignmentStatement)) {
144+
return ;
145+
}
146+
147+
const auto* lpval = ABSL_DIE_IF_NULL(
148+
manager.GetAs<verible::SyntaxTreeNode>("lpval"));
149+
150+
const auto& children = lpval->children();
151+
CHECK_EQ(children.size(), 1);
152+
const auto& lhs = *ABSL_DIE_IF_NULL(children[0]);
153+
154+
CheckAndPopScope(symbol, context);
155+
156+
if (net_reference_matcher_.Matches(lhs, &manager)) {
157+
// Matches simple LPvalue, e.g. assign a = 1'b0;
158+
const auto& lval = *ABSL_DIE_IF_NULL(
159+
manager.GetAs<verible::SyntaxTreeLeaf>("lval"));
160+
AnalyzeLHS(lval, context);
161+
} else if (net_openrange_matcher_.Matches(lhs, &manager)) {
162+
// Matches concatenated LPvalue, e.g. assign {a,b,c} = 3'b101;
163+
const auto* clist = ABSL_DIE_IF_NULL(
164+
manager.GetAs<verible::SyntaxTreeNode>("clist"));
165+
166+
for (const auto& itr : clist->children()) {
167+
if (net_expression_reference_matcher_.Matches(*itr, &manager)) {
168+
const auto& lval = *ABSL_DIE_IF_NULL(
169+
manager.GetAs<verible::SyntaxTreeLeaf>("lval"));
170+
AnalyzeLHS(lval, context);
171+
}
172+
}
173+
} // TODO: Should log if we don't get any matches?
140174
}
141175
}
142176

‎verilog/analysis/checkers/forbid_implicit_declarations_rule.h

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ class ForbidImplicitDeclarationsRule : public verible::SyntaxTreeLintRule {
6161
void CheckAndPopScope(const verible::Symbol& symbol,
6262
const verible::SyntaxTreeContext& context);
6363

64+
void AnalyzeLHS(const verible::SyntaxTreeLeaf& lval,
65+
const verible::SyntaxTreeContext& context);
66+
6467
// Pushes new scopes on stack
6568
void DetectScope(const verible::Symbol& symbol,
6669
const verible::SyntaxTreeContext& context);
@@ -92,17 +95,25 @@ class ForbidImplicitDeclarationsRule : public verible::SyntaxTreeLintRule {
9295
// Matches scopes with nets
9396
// TODO: description, other scopes
9497
const Matcher net_scope_matcher_ = verible::matcher::AnyOf(
95-
NodekModuleItemList());
98+
NodekModuleItemList(),
99+
NodekGenerateItemList());
96100

97101
// Matches nets declarations
98102
const Matcher net_declaration_matcher_ =
99103
NodekNetDeclaration(PathkNetVariableDeclarationAssign().Bind("decls"));
100104

101-
// Detects nets assignments
102-
const Matcher net_assignment_matcher_ = NodekContinuousAssignmentStatement(
103-
PathkAssignmentList(PathkNetVariableAssignment(
104-
LValueOfAssignment(
105-
PathkReference(UnqualifiedReferenceHasId().Bind("lval"))))));
105+
const Matcher net_assignment_matcher_ =
106+
NodekNetVariableAssignment(PathkLPValue().Bind("lpval"));
107+
108+
const Matcher net_openrange_matcher_ =
109+
NodekBraceGroup(PathkOpenRangeList().Bind("clist"));
110+
111+
// TODO: Any smart way to merge those two?
112+
const Matcher net_reference_matcher_ = NodekReferenceCallBase(
113+
PathkReference(UnqualifiedReferenceHasId().Bind("lval")));
114+
const Matcher net_expression_reference_matcher_ = NodekExpression(
115+
PathkReferenceCallBase(PathkReference(
116+
UnqualifiedReferenceHasId().Bind("lval"))));
106117

107118
std::set<verible::LintViolation> violations_;
108119

‎verilog/analysis/checkers/forbid_implicit_declarations_rule_test.cc

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,105 @@ TEST(ForbidImplicitDeclarationsRule, FunctionFailures) {
4343
{"module m;\nwire a1;\nbegin\nend\nassign a1 = 1'b0;\nendmodule"},
4444
{"module m;\nwire a1;\nbegin\nassign ", {kToken, "a1"}, " = 1'b0;\nend\nassign a1 = 1'b0;\nendmodule"},
4545
{"module m;\nwire a1;\nbegin\nwire a1; assign a1 = 1'b0;\nend\nassign a1 = 1'b0;\nendmodule"},
46+
47+
// multiple declarations
48+
{"module m;\nwire a0, a1;\nassign a0 = 1'b0;\nassign a1 = 1'b1;\nendmodule"},
49+
{"module m;\nwire a0, a2;\nassign a0 = 1'b0;\nassign ", {kToken, "a1"}, " = 1'b1;\nendmodule"},
50+
51+
// multiple net assignments
52+
{"module m;\nassign ", {kToken, "a"}, " = b, ", {kToken, "c"}, " = d;\nendmodule"},
53+
{"module m;\nassign ", {kToken, "a1"}, " = 1'b0;wire a1;\nendmodule"},
54+
55+
// out-of-order
56+
{"module m;\nassign ", {kToken, "a1"}, " = 1'b0;\nwire a1;\nassign a1 = 1'b1;\nendmodule"},
57+
58+
// concatenated
59+
{"module m;\nassign {", {kToken, "a"}, "} = 1'b0;\nendmodule"},
60+
{"module m;\nassign {", {kToken, "a"}, ",", {kToken, "b"}, "} = 2'b01;\nendmodule"},
61+
{"module m;\nassign {", {kToken, "a"}, ",", {kToken, "b"}, ",", {kToken, "c"}, "} = 3'b010;\nendmodule"},
62+
{"module m;\nwire b;assign {", {kToken, "a"}, ", b,", {kToken, "c"}, "} = 3'b010;\nendmodule"},
63+
64+
// around scope
65+
{"module m;assign ", {kToken, "a1"}, " = 1'b1;\nbegin\nend\nendmodule"},
66+
{"module m;begin\nend\nassign ", {kToken, "a1"}, " = 1'b1;endmodule"},
67+
68+
// declaration and assignement separated by begin-end block
69+
{"module m;\nwire a1;\nbegin\nend\nassign a1 = 1'b1;\nendmodule"},
70+
71+
// out-of-scope
72+
{"module m;\nbegin wire a1;\nend\nassign ", {kToken, "a1"}, " = 1'b1;\nendmodule"},
73+
{"module m;\nbegin wire a1;\nassign a1 = 1'b0;\nend\nassign ", {kToken, "a1"}, " = 1'b1;\nendmodule"},
74+
{"module m;\nwire a1;begin assign ", {kToken, "a1"}, " = 1'b0;\nend\nassign a1 = 1'b1;\nendmodule"},
75+
76+
// multi-level begin-end blocks
77+
{"module m;\n"
78+
" wire x1;\n"
79+
" begin\n"
80+
" wire x2;\n"
81+
" begin\n"
82+
" wire x3;\n"
83+
" begin\n"
84+
" wire x4;\n"
85+
" assign x4 = 1'b0;\n"
86+
" assign ", {kToken, "x3"}, " = 1'b0;\n"
87+
" assign ", {kToken, "x2"}, " = 1'b0;\n"
88+
" assign ", {kToken, "x1"}, " = 1'b0;\n"
89+
" end\n"
90+
" assign ", {kToken, "x4"}, " = 1'b0;\n"
91+
" assign x3 = 1'b1;\n"
92+
" assign ", {kToken, "x2"}, " = 1'b0;\n"
93+
" assign ", {kToken, "x1"}, " = 1'b0;\n"
94+
" end\n"
95+
" assign ", {kToken, "x4"}, " = 1'b0;\n"
96+
" assign ", {kToken, "x3"}, " = 1'b0;\n"
97+
" assign x2 = 1'b0;\n"
98+
" assign ", {kToken, "x1"}, " = 1'b0;\n"
99+
" end\n"
100+
" assign ", {kToken, "x4"}, " = 1'b0;\n"
101+
" assign ", {kToken, "x3"}, " = 1'b0;\n"
102+
" assign ", {kToken, "x2"}, " = 1'b0;\n"
103+
" assign x1 = 1'b1;\n"
104+
"endmodule"},
105+
106+
// generate block, TODO: multi-level
107+
{"module m;\ngenerate\nendgenerate\nendmodule"},
108+
{"module m;\n"
109+
" wire a1;\n"
110+
" assign a1 = 1'b1;\n"
111+
" generate\n"
112+
" endgenerate\n"
113+
" assign a1 = 1'b0;\n"
114+
"endmodule"},
115+
{"module m;\n"
116+
" generate\n"
117+
" wire a1;\n"
118+
" assign a1 = 1'b1;\n"
119+
" endgenerate\n"
120+
"endmodule"},
121+
{"module m;\n"
122+
" generate\n"
123+
" assign ", {kToken, "a1"}, " = 1'b1;\n"
124+
" endgenerate\n"
125+
"endmodule"},
126+
{"module m;\n"
127+
" generate\n"
128+
" wire a1;\n"
129+
" assign a1 = 1'b1;\n"
130+
" endgenerate\n"
131+
" assign ", {kToken, "a1"}, " = 1'b1;\n"
132+
"endmodule"},
133+
{"module m;\n"
134+
" wire a1;\n"
135+
" assign a1 = 1'b1;\n"
136+
" generate\n"
137+
" assign ", {kToken, "a1"}, " = 1'b1;\n"
138+
" endgenerate\n"
139+
" assign a1 = 1'b0;\n"
140+
"endmodule"},
141+
142+
// TODO: module scope
143+
144+
// TODO: nets declared inside terminal/port connection list
46145
};
47146

48147
RunLintTestCases<VerilogAnalyzer, ForbidImplicitDeclarationsRule>(

0 commit comments

Comments
 (0)