Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide autofixes for always_ff_non_blocking_rule #1912

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions verible/common/analysis/linter-test-utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include <functional>
#include <initializer_list>
#include <iosfwd>
#include <iterator>
#include <memory>
#include <set>
#include <sstream>
Expand Down Expand Up @@ -114,9 +115,12 @@ void RunLintTestCases(std::initializer_list<LintTestCase> tests,
}

struct AutoFixInOut {
static constexpr int NO_FIX_AVAILABLE = -1;

absl::string_view code;
absl::string_view expected_output;
int fix_alternative = 0; // Some rules provide alternative fixes
int violation_number = 0;
};

// Tests that LintTestCase test has expected violations under make_rule
Expand All @@ -134,10 +138,17 @@ void RunLintAutoFixCase(const AutoFixInOut &test,
const LintRuleStatus rule_status = lint_runner.Run(analyzer.Data(), filename);
const auto &violations(rule_status.violations);

CHECK_EQ(violations.size(), 1) << "TODO: apply multi-violation fixes";
CHECK_GT(violations.begin()->autofixes.size(), test.fix_alternative);
const verible::AutoFix &fix =
rule_status.violations.begin()->autofixes[test.fix_alternative];
CHECK_GT(violations.size(), test.violation_number);
const LintViolation &violation =
*std::next(violations.begin(), test.violation_number);

if (test.fix_alternative == AutoFixInOut::NO_FIX_AVAILABLE) {
CHECK_EQ(violation.autofixes.size(), 0);
return;
}

CHECK_GT(violation.autofixes.size(), test.fix_alternative);
const verible::AutoFix &fix = violation.autofixes[test.fix_alternative];
std::string fix_out = fix.Apply(analyzer.Data().Contents());

EXPECT_EQ(test.expected_output, fix_out) << "For input " << test.code;
Expand Down
30 changes: 30 additions & 0 deletions verible/verilog/CST/statement.cc
Original file line number Diff line number Diff line change
Expand Up @@ -535,4 +535,34 @@ const verible::SyntaxTreeNode *GetIfHeaderExpression(
return verible::GetSubtreeAsNode(*paren_group, NodeEnum::kParenGroup, 1);
}

const verible::SyntaxTreeLeaf *GetAssignModifyOperator(
const verible::SyntaxTreeNode &assign_modify) {
return verible::GetSubtreeAsLeaf(assign_modify,
NodeEnum::kAssignModifyStatement, 1);
}

const verible::SyntaxTreeNode *GetAssignModifyRhs(
const verible::SyntaxTreeNode &assign_modify) {
return verible::GetSubtreeAsNode(assign_modify,
NodeEnum::kAssignModifyStatement, 2);
}

const verible::SyntaxTreeNode *GetAssignModifyLhs(
const verible::SyntaxTreeNode &assign_modify) {
return verible::GetSubtreeAsNode(assign_modify,
NodeEnum::kAssignModifyStatement, 0);
}

const verible::SyntaxTreeNode *GetNetVariableAssignmentLhs(
const verible::SyntaxTreeNode &assignment) {
return verible::GetSubtreeAsNode(assignment, NodeEnum::kNetVariableAssignment,
0);
}

const verible::SyntaxTreeLeaf *GetNetVariableAssignmentOperator(
const verible::SyntaxTreeNode &assignment) {
return verible::GetSubtreeAsLeaf(assignment, NodeEnum::kNetVariableAssignment,
1);
}

} // namespace verilog
25 changes: 25 additions & 0 deletions verible/verilog/CST/statement.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,31 @@ const verible::SyntaxTreeNode *GetIfClauseHeader(
const verible::SyntaxTreeNode *GetIfHeaderExpression(
const verible::SyntaxTreeNode &if_header);

// Return the operator from an AssignModifyStatement
// Example: get '&' from 'x &= y'
const verible::SyntaxTreeLeaf *GetAssignModifyOperator(
const verible::SyntaxTreeNode &assign_modify);

// Return the right hand side (Rhs) from an AssignModifyStatement
// Example: get 'y' from 'x &= y'
const verible::SyntaxTreeNode *GetAssignModifyRhs(
const verible::SyntaxTreeNode &assign_modify);

// Return the left hand side (Lhs) from an AssignModifyStatement
// Example: get 'x' from 'x &= y'
const verible::SyntaxTreeNode *GetAssignModifyLhs(
const verible::SyntaxTreeNode &assign_modify);

// Return the left hand side (Lhs) from a NetVariableAssignment
// Example: get 'x' from 'x = y'
const verible::SyntaxTreeNode *GetNetVariableAssignmentLhs(
const verible::SyntaxTreeNode &assignment);

// Return the operator from a NetVariableAssignment
// Example: get '=' from 'x = y'
const verible::SyntaxTreeLeaf *GetNetVariableAssignmentOperator(
const verible::SyntaxTreeNode &assignment);

} // namespace verilog

#endif // VERIBLE_VERILOG_CST_STATEMENT_H_
253 changes: 253 additions & 0 deletions verible/verilog/CST/statement_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1585,5 +1585,258 @@ TEST(GetIfClauseExpressionTest, Various) {
}
}

TEST(GetAssignModifyOperator, Various) {
constexpr int kTag = 1; // value doesn't matter
const SyntaxTreeSearchTestCase kTestCases[] = {
{""},
{"module m;\nendmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k ",
{kTag, "&="},
" 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k ",
{kTag, "|="},
" 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k ",
{kTag, "+="},
" 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k = 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_ff @(posedge clk) begin "
"k <= 1;\nend\n"
"endmodule\n"},
};
for (const auto &test : kTestCases) {
TestVerilogSyntaxRangeMatches(
__FUNCTION__, test, [](const TextStructureView &text_structure) {
const auto &root = text_structure.SyntaxTree();
const auto &assign_modify_statements = SearchSyntaxTree(
*ABSL_DIE_IF_NULL(root), NodekAssignModifyStatement());

std::vector<TreeSearchMatch> operators;
for (const auto &block : assign_modify_statements) {
const auto *operator_ = GetAssignModifyOperator(
verible::SymbolCastToNode(*block.match));
operators.emplace_back(
TreeSearchMatch{operator_, {/* ignored context */}});
}
return operators;
});
}
}

TEST(GetAssignModifyLhs, Various) {
constexpr int kTag = 1; // value doesn't matter
const SyntaxTreeSearchTestCase kTestCases[] = {
{""},
{"module m;\nendmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin ",
{kTag, "k"},
" &= 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n",
{kTag, "k"},
" |= 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n",
{kTag, "k"},
" += 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k = 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"always_comb begin\n"
"reg k = 1;\nend\n"
"endmodule\n"},
};
for (const auto &test : kTestCases) {
TestVerilogSyntaxRangeMatches(
__FUNCTION__, test, [](const TextStructureView &text_structure) {
const auto &root = text_structure.SyntaxTree();
const auto &assign_modify_statements = SearchSyntaxTree(
*ABSL_DIE_IF_NULL(root), NodekAssignModifyStatement());

std::vector<TreeSearchMatch> left_hand_sides;
for (const auto &block : assign_modify_statements) {
const auto *lhs =
GetAssignModifyLhs(verible::SymbolCastToNode(*block.match));
left_hand_sides.emplace_back(
TreeSearchMatch{lhs, {/* ignored context */}});
}
return left_hand_sides;
});
}
}

TEST(GetAssignModifyRhs, Various) {
constexpr int kTag = 1; // value doesn't matter
const SyntaxTreeSearchTestCase kTestCases[] = {
{""},
{"module m;\nendmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin "
"k &= ",
{kTag, "1"},
";\nend\nendmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k |= ",
{kTag, "(1 + 2)"},
";\nend\nendmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k += ",
{kTag, "1"},
";\nend\nendmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k = 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"always_comb begin\n"
"reg k = 1;\nend\n"
"endmodule\n"},
};
for (const auto &test : kTestCases) {
TestVerilogSyntaxRangeMatches(
__FUNCTION__, test, [](const TextStructureView &text_structure) {
const auto &root = text_structure.SyntaxTree();
const auto &assign_modify_statements = SearchSyntaxTree(
*ABSL_DIE_IF_NULL(root), NodekAssignModifyStatement());

std::vector<TreeSearchMatch> right_hand_sides;
for (const auto &block : assign_modify_statements) {
const auto *rhs =
GetAssignModifyRhs(verible::SymbolCastToNode(*block.match));
right_hand_sides.emplace_back(
TreeSearchMatch{rhs, {/* ignored context */}});
}
return right_hand_sides;
});
}
}

TEST(GetNetVariableAssignmentLhs, Various) {
constexpr int kTag = 1; // value doesn't matter
const SyntaxTreeSearchTestCase kTestCases[] = {
{""},
{"module m;\nendmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n",
{kTag, "k"},
" = 1;\nend\n",
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n",
"k &= 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k |= 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_ff begin\n"
"k <= 1;\nend\n"
"endmodule\n"},
};
for (const auto &test : kTestCases) {
TestVerilogSyntaxRangeMatches(
__FUNCTION__, test, [](const TextStructureView &text_structure) {
const auto &root = text_structure.SyntaxTree();
const auto &net_var_assignments = SearchSyntaxTree(
*ABSL_DIE_IF_NULL(root), NodekNetVariableAssignment());

std::vector<TreeSearchMatch> left_hand_sides;
for (const auto &assignment : net_var_assignments) {
const auto *lhs = GetNetVariableAssignmentLhs(
verible::SymbolCastToNode(*assignment.match));
left_hand_sides.emplace_back(
TreeSearchMatch{lhs, {/* ignored context */}});
}
return left_hand_sides;
});
}
}

TEST(GetNetVariableAssignmentOperator, Various) {
constexpr int kTag = 1; // value doesn't matter
const SyntaxTreeSearchTestCase kTestCases[] = {
{""},
{"module m;\nendmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n",
"k ",
{kTag, "="},
" 1;\nend\n",
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n",
"k &= 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_comb begin\n"
"k |= 1;\nend\n"
"endmodule\n"},
{"module m;\n"
"reg k;\n"
"always_ff begin\n"
"k <= 1;\nend\n"
"endmodule\n"},
};
for (const auto &test : kTestCases) {
TestVerilogSyntaxRangeMatches(
__FUNCTION__, test, [](const TextStructureView &text_structure) {
const auto &root = text_structure.SyntaxTree();
const auto &net_var_assignments = SearchSyntaxTree(
*ABSL_DIE_IF_NULL(root), NodekNetVariableAssignment());

std::vector<TreeSearchMatch> left_hand_sides;
for (const auto &assignment : net_var_assignments) {
const auto *lhs = GetNetVariableAssignmentOperator(
verible::SymbolCastToNode(*assignment.match));
left_hand_sides.emplace_back(
TreeSearchMatch{lhs, {/* ignored context */}});
}
return left_hand_sides;
});
}
}

} // namespace
} // namespace verilog
4 changes: 4 additions & 0 deletions verible/verilog/analysis/checkers/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1229,13 +1229,17 @@ cc_library(
"//verible/common/text:config-utils",
"//verible/common/text:symbol",
"//verible/common/text:syntax-tree-context",
"//verible/common/text:tree-utils",
"//verible/common/util:casts",
"//verible/common/util:logging",
"//verible/verilog/CST:expression",
"//verible/verilog/CST:statement",
"//verible/verilog/CST:verilog-matchers",
"//verible/verilog/CST:verilog-nonterminals",
"//verible/verilog/analysis:descriptions",
"//verible/verilog/analysis:lint-rule-registry",
"@com_google_absl//absl/status",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:string_view",
],
alwayslink = 1,
Expand Down
Loading
Loading