Skip to content

Commit

Permalink
Merge pull request #1628 from antmicro/msrs/change-syntax-of-attribut…
Browse files Browse the repository at this point in the history 
…es_v2

Simplify syntax of attributes
  • Loading branch information
@hzeller
hzeller authored Feb 15, 2023
2 parents 42c9491 + a108c4c commit 1a39cce
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 44 deletions.
9 changes: 9 additions & 0 deletions verilog/CST/verilog_matchers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,15 @@ static const auto AlwaysStatementHasEventControlStar =
verible::matcher::MakePathMatcher(
{N(kProceduralTimingControlStatement), N(kEventControl), L('*')});

static const auto AlwaysStatementHasEventControlStarAndParentheses =
verible::matcher::MakePathMatcher({N(kProceduralTimingControlStatement),
N(kEventControl), N(kParenGroup),
L('*')});

static const auto AlwaysStatementHasParentheses =
verible::matcher::MakePathMatcher({N(kProceduralTimingControlStatement),
N(kEventControl), N(kParenGroup)});

// Matches occurrence of the 'always' keyword.
// This is needed to distinguish between various kAlwaysStatement's.
// This matches 'always', but not 'always_ff', nor 'always_comb'.
Expand Down
39 changes: 37 additions & 2 deletions verilog/CST/verilog_matchers_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -510,8 +510,6 @@ TEST(VerilogMatchers, AlwaysStatementHasEventControlStarTests) {
EmbedInModule("always_comb begin a = b; end"), 0},
{AlwaysStatementHasEventControlStar(),
EmbedInModule("always @* begin a = b; end"), 1},
{AlwaysStatementHasEventControlStar(),
EmbedInModule("always @(*) begin a = b; end"), 1},
{AlwaysStatementHasEventControlStar(),
EmbedInModule("always @(posedge foo) begin a <= b; end"), 0},
{AlwaysStatementHasEventControlStar(),
Expand All @@ -523,6 +521,43 @@ TEST(VerilogMatchers, AlwaysStatementHasEventControlStarTests) {
verible::matcher::RunRawMatcherTestCase<VerilogAnalyzer>(test);
}

TEST(VerilogMatchers, AlwaysStatementHasEventControlStarAndParentheses) {
const RawMatcherTestCase tests[] = {
{AlwaysStatementHasEventControlStarAndParentheses(),
EmbedInModule("always @* begin a = b; end"), 0},
{AlwaysStatementHasEventControlStarAndParentheses(),
EmbedInModule("always @(*) begin a = b; end"), 1},
{AlwaysStatementHasEventControlStarAndParentheses(),
EmbedInModule("always @( *) begin a = b; end"), 1},
{AlwaysStatementHasEventControlStarAndParentheses(),
EmbedInModule("always @(* ) begin a = b; end"), 1},
{AlwaysStatementHasEventControlStarAndParentheses(),
EmbedInModule("always @( * ) begin a = b; end"), 1},
};
for (const auto& test : tests)
verible::matcher::RunRawMatcherTestCase<VerilogAnalyzer>(test);
}

// Tests for AlwaysStatementHasParentheses matching
TEST(VerilogMatchers, AlwaysStatementHasParentheses) {
const RawMatcherTestCase tests[] = {
{AlwaysStatementHasParentheses(),
EmbedInModule("always @* begin a = b; end"), 0},
{AlwaysStatementHasParentheses(),
EmbedInModule("always @(*) begin a = b; end"), 1},
{AlwaysStatementHasParentheses(),
EmbedInModule("always @( *) begin a = b; end"), 1},
{AlwaysStatementHasParentheses(),
EmbedInModule("always @(* ) begin a = b; end"), 1},
{AlwaysStatementHasParentheses(),
EmbedInModule("always @( * ) begin a = b; end"), 1},
{AlwaysStatementHasParentheses(),
EmbedInModule("always @(posedge foo) begin a <= b; end"), 1},
};
for (const auto& test : tests)
verible::matcher::RunRawMatcherTestCase<VerilogAnalyzer>(test);
}

// Tests that matcher finds left-hand-sides of assignments.
TEST(VerilogMatchers, PathkLPValueTests) {
const RawMatcherTestCase tests[] = {
Expand Down
9 changes: 9 additions & 0 deletions verilog/analysis/checkers/always_comb_rule.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,22 @@ static const Matcher& AlwaysStarMatcher() {
return matcher;
}

static const Matcher& AlwaysStarMatcherWithParentheses() {
static const Matcher matcher(NodekAlwaysStatement(
AlwaysKeyword(), AlwaysStatementHasEventControlStarAndParentheses()));
return matcher;
}

void AlwaysCombRule::HandleSymbol(const verible::Symbol& symbol,
const SyntaxTreeContext& context) {
// Check for offending use of always @*
verible::matcher::BoundSymbolManager manager;
if (AlwaysStarMatcher().Matches(symbol, &manager)) {
violations_.insert(LintViolation(symbol, kMessage, context));
}
if (AlwaysStarMatcherWithParentheses().Matches(symbol, &manager)) {
violations_.insert(LintViolation(symbol, kMessage, context));
}
}

LintRuleStatus AlwaysCombRule::Report() const {
Expand Down
8 changes: 8 additions & 0 deletions verilog/analysis/checkers/always_comb_rule_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ TEST(AlwaysCombTest, FunctionFailures) {
{"module m;\ninitial begin end\nendmodule"},
{"module m;\n", {kToken, "always"}, " @* begin end\nendmodule"},
{"module m;\n", {kToken, "always"}, " @(*) begin end\nendmodule"},
{"module m;\n", {kToken, "always"}, " @( *) begin end\nendmodule"},
{"module m;\n", {kToken, "always"}, " @(* ) begin end\nendmodule"},
{"module m;\n", {kToken, "always"}, " @( * ) begin end\nendmodule"},
{"module m;\n", {kToken, "always"}, " @(/*t*/*) begin end\nendmodule"},
{"module m;\n", {kToken, "always"}, " @(*/*t*/) begin end\nendmodule"},
{"module m;\n",
{kToken, "always"},
" @(/*t*/*/*t*/) begin end\nendmodule"},
{"module m;\nalways_ff begin a <= b; end\nendmodule"},
{"module m;\nalways_comb begin a = b; end\nendmodule"},
};
Expand Down
32 changes: 32 additions & 0 deletions verilog/formatting/formatter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15356,6 +15356,38 @@ static constexpr FormatterTestCase kFormatterTestCases[] = {
" logic/*t*/ [0 : 1] /*t*/ a; /*t*/\n"
" T1/*t*/ [0 : 1] /*t*/ c; /*t*/\n"
"endclass\n"},
{"always @(*/*t*/) begin\n"
"end\n",
"always @(* /*t*/) begin\n"
"end\n"},
{"always @(/*t*/*) begin\n"
"end\n",
"always @( /*t*/ *) begin\n"
"end\n"},
{"always @(/*t*/*/*t*/) begin\n"
"end\n",
"always @( /*t*/ * /*t*/) begin\n"
"end\n"},
{"always @(*) begin\n"
"end\n",
"always @(*) begin\n"
"end\n"},
{"always @(* ) begin\n"
"end\n",
"always @(*) begin\n"
"end\n"},
{"always @( *) begin\n"
"end\n",
"always @(*) begin\n"
"end\n"},
{"always @( * ) begin\n"
"end\n",
"always @(*) begin\n"
"end\n"},
{"always @( /*t*/ * /*t*/ ) begin\n"
"end\n",
"always @( /*t*/ * /*t*/) begin\n"
"end\n"},

// -----------------------------------------------------------------
};
Expand Down
34 changes: 3 additions & 31 deletions verilog/parser/verilog.lex
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,8 @@ AngleBracketInclude {UnterminatedAngleBracketString}>
/* attribute lists, treated like comments */
AttributesBegin "(*"
AttributesEnd [*]+")"
/* was TK_PSTAR and TK_STARP */
AttributesContinue [^ \r\n\t\f\b)]
AttributesContent ([^*]|("*"+[^)*]))*
AttributesEnd "*)"
AttributesContent ([^)]|("("[^*]*")"))*
Attributes {AttributesBegin}{AttributesContent}{AttributesEnd}
/* comments */
Expand Down Expand Up @@ -859,36 +857,10 @@ zi_zp { UpdateLocation(); return TK_zi_zp; }
"[->" { UpdateLocation(); return TK_LBRARROW; }
"@@" { UpdateLocation(); return TK_ATAT; }

/* Watch out for the tricky case of (*). Cannot parse this as "(*"
and ")", but since I know that this is really ( * ), replace it
with "*" and return that. */
/* TODO(fangism): see if this can be simplified without lexer states. */
{AttributesBegin} {
yy_push_state(ATTRIBUTE_START);
yymore();
}
<ATTRIBUTE_START>{Space}+ {
yymore();
}
<ATTRIBUTE_START>{LineTerminator} {
yymore();
}
<ATTRIBUTE_START>")" {
/* This is the (*) case. */
yy_pop_state();
UpdateLocation();
return '*';
}
<ATTRIBUTE_START,ATTRIBUTE_MIDDLE>{AttributesEnd} {
yy_pop_state();
{Attributes} {
UpdateLocation();
return TK_ATTRIBUTE;
}
<ATTRIBUTE_START>{AttributesContinue} {
yy_set_top_state(ATTRIBUTE_MIDDLE);
yymore();
}
<ATTRIBUTE_MIDDLE>{AttributesContent} { yymore(); }

/* Only enter the EDGES state if the next token is '[', otherwise, rewind. */
<EDGES_POSSIBLY>{
Expand Down
1 change: 0 additions & 1 deletion verilog/parser/verilog.y
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4128,7 +4128,6 @@ event_control
{ $$ = MakeTaggedNode(N::kEventControl, $1, MakeParenGroup($2, $3, $4));}
| '@' '*'
{ $$ = MakeTaggedNode(N::kEventControl, $1, $2);}
/* same as @ (*), which the lexer returns as just '*' */
;
event_control_opt
: event_control
Expand Down
23 changes: 13 additions & 10 deletions verilog/parser/verilog_lexer_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,20 @@ static std::initializer_list<LexerTestData> kCommentTests = {
// treating attributes lists as C-style comments,
// except they are not returned as comment blocks.
static std::initializer_list<SimpleTestData> kAttributeTests = {
{"(**)"}, {"(* *)"},
{"(* x)*)"}, {"(* ** *)"},
{"(***)"}, {"(** **)"},
{"(*\n*)"}, {"(* style=flat *)"},
{"(*foo=bar*)"}, {"(* style=flat, fill=empty *)"},
{"(**)"},
{"(* *)"},
{"(* ** *)"},
{"(***)"},
{"(** **)"},
{"(*\n*)"},
{"(* style=flat *)"},
{"(*foo=bar*)"},
{"(*foo=bar()*)"},
{"(*foo=\"bar()\"*)"},
{"(*foo=\"bar(abc)\"*)"},
{"(*foo='{\"bar_.*\"} *)"},
{"(* style=flat, fill=empty *)"},
};

static std::initializer_list<LexerTestData> kAttributeSequenceTests = {
{{TK_ATTRIBUTE, "(**)"}, {TK_ATTRIBUTE, "(**)"}},
{{TK_ATTRIBUTE, "(* style=flat,\nfill=empty *)"}, {TK_NEWLINE, "\n"}},
Expand Down Expand Up @@ -1362,7 +1369,6 @@ static std::initializer_list<SimpleTestData> kEvalStringLiteralTests = {

// tokens with special handling in lexer
static std::initializer_list<LexerTestData> kTrickyTests = {
{{'*', "(*)"}},
{{TK_COLON_DIV, ":/"}, {TK_SPACE, " "}},
{{TK_COLON_DIV, ":/"}, {TK_DecNumber, "8"}},
{':', {TK_EOL_COMMENT, "//"}, {TK_NEWLINE, "\n"}},
Expand Down Expand Up @@ -1426,9 +1432,6 @@ static std::initializer_list<LexerTestData> kSequenceTests = {
{{MacroNumericWidth, "`WIDTH"},
{TK_BinBase, "'b"},
{MacroIdentifier, "`DIGITS"}},
{{'*', "(*)"}, {'*', "(*)"}},
{{'*', "(*)"}, " ", {'*', "(*)"}},
{{'*', "(* )"}, {'*', "(* )"}},
};

static std::initializer_list<LexerTestData> kContextKeywordTests = {
Expand Down

0 comments on commit 1a39cce

Please sign in to comment.