From 589775538c23975d79aa21050557a37b76acb1dd Mon Sep 17 00:00:00 2001 From: Eddie Hung Date: Mon, 11 May 2020 09:33:11 -0700 Subject: [PATCH 1/7] tests: add #2037 testcase --- tests/verilog/bug2037.ys | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 tests/verilog/bug2037.ys diff --git a/tests/verilog/bug2037.ys b/tests/verilog/bug2037.ys new file mode 100644 index 000000000..afe92022e --- /dev/null +++ b/tests/verilog/bug2037.ys @@ -0,0 +1,9 @@ +logger -expect warning "Attribute\(s\) attached to null statement\. Ignoring\." 1 +logger -expect-no-warnings +read_verilog < Date: Mon, 11 May 2020 09:33:19 -0700 Subject: [PATCH 2/7] verilog: fix #2037 by permitting (and freeing) attributes on null stmt --- frontends/verilog/verilog_parser.y | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index d39b72547..a0250439e 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -2228,7 +2228,11 @@ simple_behavioral_stmt: behavioral_stmt: defattr | assert | wire_decl | param_decl | localparam_decl | typedef_decl | non_opt_delay behavioral_stmt | - attr simple_behavioral_stmt ';' | ';' | + attr simple_behavioral_stmt ';' | + attr ';' { + log_file_warning(current_filename, get_line_num(), "Attribute(s) attached to null statement. Ignoring.\n"); + free_attr($1); + } | attr hierarchical_id { AstNode *node = new AstNode(AST_TCALL); node->str = *$2; From 88bddb37c91e8fe136e5c9cc2ade20fadccd1946 Mon Sep 17 00:00:00 2001 From: Eddie Hung Date: Mon, 11 May 2020 10:20:33 -0700 Subject: [PATCH 3/7] verilog: handle empty generate statement by removing gen_stmt_or_null... ... rule which causes a s/r conflict. Now we get an empty genblock, which should be okay. --- frontends/verilog/verilog_parser.y | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index a0250439e..eb7e136ae 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -2440,7 +2440,7 @@ gen_case_item: } case_select { case_type_stack.push_back(0); SET_AST_NODE_LOC(ast_stack.back(), @2, @2); - } gen_stmt_or_null { + } gen_stmt_block { case_type_stack.pop_back(); ast_stack.pop_back(); }; @@ -2532,7 +2532,11 @@ module_gen_body: /* empty */; gen_stmt_or_module_body_stmt: - gen_stmt | module_body_stmt; + gen_stmt | module_body_stmt | + attr ';' { + log_file_warning(current_filename, get_line_num(), "Attribute(s) attached to null statement. Ignoring.\n"); + free_attr($1); + }; // this production creates the obligatory if-else shift/reduce conflict gen_stmt: @@ -2554,7 +2558,7 @@ gen_stmt: AstNode *block = new AstNode(AST_GENBLOCK); ast_stack.back()->children.push_back(block); ast_stack.push_back(block); - } gen_stmt_or_null { + } gen_stmt_block { ast_stack.pop_back(); } opt_gen_else { SET_AST_NODE_LOC(ast_stack.back(), @1, @7); @@ -2604,11 +2608,8 @@ gen_stmt_block: ast_stack.pop_back(); }; -gen_stmt_or_null: - gen_stmt_block | ';'; - opt_gen_else: - TOK_ELSE gen_stmt_or_null | /* empty */ %prec FAKE_THEN; + TOK_ELSE gen_stmt_block | /* empty */ %prec FAKE_THEN; expr: basic_expr { From 29d84339bf9ec8f1d2be3fa20f81843f3ee08324 Mon Sep 17 00:00:00 2001 From: Eddie Hung Date: Mon, 11 May 2020 10:26:08 -0700 Subject: [PATCH 4/7] tests: add an generate-else test too --- tests/verilog/bug2037.ys | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/verilog/bug2037.ys b/tests/verilog/bug2037.ys index afe92022e..42c4b8f5d 100644 --- a/tests/verilog/bug2037.ys +++ b/tests/verilog/bug2037.ys @@ -7,3 +7,37 @@ module test (); if (y) (* foo *) ; endmodule EOT + + +design -reset +logger -expect warning "Attribute\(s\) attached to null statement\. Ignoring\." 3 # cumulative +logger -expect-no-warnings +read_verilog < Date: Thu, 14 May 2020 10:46:40 -0700 Subject: [PATCH 5/7] verilog: do not warn for attributes on null statements --- frontends/verilog/verilog_parser.y | 2 -- tests/verilog/bug2037.ys | 8 ++++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index eb7e136ae..475557af9 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -2230,7 +2230,6 @@ behavioral_stmt: non_opt_delay behavioral_stmt | attr simple_behavioral_stmt ';' | attr ';' { - log_file_warning(current_filename, get_line_num(), "Attribute(s) attached to null statement. Ignoring.\n"); free_attr($1); } | attr hierarchical_id { @@ -2534,7 +2533,6 @@ module_gen_body: gen_stmt_or_module_body_stmt: gen_stmt | module_body_stmt | attr ';' { - log_file_warning(current_filename, get_line_num(), "Attribute(s) attached to null statement. Ignoring.\n"); free_attr($1); }; diff --git a/tests/verilog/bug2037.ys b/tests/verilog/bug2037.ys index 42c4b8f5d..eb4f0fac4 100644 --- a/tests/verilog/bug2037.ys +++ b/tests/verilog/bug2037.ys @@ -1,4 +1,3 @@ -logger -expect warning "Attribute\(s\) attached to null statement\. Ignoring\." 1 logger -expect-no-warnings read_verilog < Date: Thu, 14 May 2020 16:32:14 -0700 Subject: [PATCH 6/7] test: add attribute-before-stmt test from @nakengelhardt --- tests/verilog/bug2037.ys | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/verilog/bug2037.ys b/tests/verilog/bug2037.ys index eb4f0fac4..4b629ba92 100644 --- a/tests/verilog/bug2037.ys +++ b/tests/verilog/bug2037.ys @@ -41,3 +41,18 @@ module test (); endmodule EOT select -assert-none a:* + + +design -reset +read_verilog < Date: Thu, 21 May 2020 09:46:26 -0700 Subject: [PATCH 7/7] verilog: move attr from simple_behav_stmt to its children to attach --- frontends/verilog/verilog_parser.y | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y index 475557af9..c8223f41d 100644 --- a/frontends/verilog/verilog_parser.y +++ b/frontends/verilog/verilog_parser.y @@ -2203,32 +2203,36 @@ assert_property: }; simple_behavioral_stmt: - lvalue '=' delay expr { - AstNode *node = new AstNode(AST_ASSIGN_EQ, $1, $4); + attr lvalue '=' delay expr { + AstNode *node = new AstNode(AST_ASSIGN_EQ, $2, $5); ast_stack.back()->children.push_back(node); - SET_AST_NODE_LOC(node, @1, @4); + SET_AST_NODE_LOC(node, @2, @5); + append_attr(node, $1); } | - lvalue TOK_INCREMENT { - AstNode *node = new AstNode(AST_ASSIGN_EQ, $1, new AstNode(AST_ADD, $1->clone(), AstNode::mkconst_int(1, true))); + attr lvalue TOK_INCREMENT { + AstNode *node = new AstNode(AST_ASSIGN_EQ, $2, new AstNode(AST_ADD, $2->clone(), AstNode::mkconst_int(1, true))); ast_stack.back()->children.push_back(node); - SET_AST_NODE_LOC(node, @1, @2); + SET_AST_NODE_LOC(node, @2, @3); + append_attr(node, $1); } | - lvalue TOK_DECREMENT { - AstNode *node = new AstNode(AST_ASSIGN_EQ, $1, new AstNode(AST_SUB, $1->clone(), AstNode::mkconst_int(1, true))); + attr lvalue TOK_DECREMENT { + AstNode *node = new AstNode(AST_ASSIGN_EQ, $2, new AstNode(AST_SUB, $2->clone(), AstNode::mkconst_int(1, true))); ast_stack.back()->children.push_back(node); - SET_AST_NODE_LOC(node, @1, @2); + SET_AST_NODE_LOC(node, @2, @3); + append_attr(node, $1); } | - lvalue OP_LE delay expr { - AstNode *node = new AstNode(AST_ASSIGN_LE, $1, $4); + attr lvalue OP_LE delay expr { + AstNode *node = new AstNode(AST_ASSIGN_LE, $2, $5); ast_stack.back()->children.push_back(node); - SET_AST_NODE_LOC(node, @1, @4); + SET_AST_NODE_LOC(node, @2, @5); + append_attr(node, $1); }; // this production creates the obligatory if-else shift/reduce conflict behavioral_stmt: defattr | assert | wire_decl | param_decl | localparam_decl | typedef_decl | non_opt_delay behavioral_stmt | - attr simple_behavioral_stmt ';' | + simple_behavioral_stmt ';' | attr ';' { free_attr($1); } |