Support parsing event expression in sequence

[mlyoung101]

Example:

module foo(
    input logic clk
);
  sequence sync_posedge;
     @(posedge clk) 1;
  endsequence
endmodule

This produces the following error:

$ verilator --lint-only test.sv
%Error: test.sv:6:6: syntax error, unexpected '@', expecting IDENTIFIER-for-type
    6 |      @(posedge clk) 1;
      |      ^
        ... See the manual at https://verilator.org/verilator_doc.html?v=5.049 for more assistance.
%Error: Cannot continue
        ... This fatal error may be caused by the earlier error(s); resolve those first.

However, I believe this is valid SV and it parses fine under Slang:

$ slang test.sv
Top level design units:
    foo

Build succeeded: 0 errors, 0 warnings

Verilator version: Verilator 5.049 devel rev v5.048-44-gd5d799bdd

OS: Arch Linux

[mlyoung101]

My understanding is that sequences are meant to be supported as #4425 was closed?

[wsnyder]

#4425 was about supporting sequence in general, some pieces are still lacking unfortunately.

Generally, anything unsupported will give an UNSUPPORTED error, unfortunately there are a couple of parser limitations due to the complexity of the IEEE grammar that haven't been implemented yet that give syntax errors instead, and this is one.

sexpr<nodeExprp>:  // ==IEEE: sequence_expr  (The name sexpr is important as regexps just add an "s" to expr.)
        ...
        //UNSUP clocking_event ~p~sexpr %prec prSEQ_CLOCKING    { }

Welcome a pull to fix this. Note it's not as simple as removing the comment due to the grammar conflict, or we'd already support it.

[mlyoung101]

Cool, that makes sense, no worries. Yes, I would like to work on this soon, I'll just have to add it to my list, could be a little while unfortunately and my knowledge of yacc is very shaky at best. I'll ping again once I make some progress.

[wsnyder]

Here's the parser change needed. The Ast and production rule still needs updating to make this work.

Parser changes

diff --git a/src/verilog.y b/src/verilog.y
index ab969b77b..dbfef4161 100644
--- a/src/verilog.y
+++ b/src/verilog.y
@@ -889,13 +889,14 @@ BISONPRE_VERSION(3.7,%define api.header.include {"V3ParseBison.h"})
 //UNSUP %token<fl>      prREDUCTION
 //UNSUP %token<fl>      prNEGATION
 //UNSUP %token<fl>      prEVENTBEGIN
+%token<fl>      prSEQ_CLOCKING
 %token<fl>      prTAGGED

 // These prevent other conflicts
 %left           yP_ANDANDAND
 %left           yMATCHES
 %left           prTAGGED
-//UNSUP %left   prSEQ_CLOCKING
+%left           prSEQ_CLOCKING

 // PSL op precedence

@@ -6656,23 +6657,7 @@ sequence_declarationBody<nodep>:  // IEEE: part of sequence_declaration
         ;

 property_spec<propSpecp>:               // IEEE: property_spec
-        //UNSUP: This rule has been super-specialized to what is supported now
-        //UNSUP remove below
-                '@' '(' senitem ')' yDISABLE yIFF '(' expr ')' pexpr
-                        { $$ = new AstPropSpec{$1, $3, $8, $10}; }
-        |       '@' '(' senitem ')' pexpr
-                        { $$ = new AstPropSpec{$1, $3, nullptr, $5}; }
-        |       '@' senitemVar pexpr
-                        { $$ = new AstPropSpec{$1, $2, nullptr, $3}; }
-        //                      // Disable applied after the event occurs,
-        //                      // so no existing AST can represent this
-        |       yDISABLE yIFF '(' expr ')' '@' '(' senitemEdge ')' pexpr
-                        { $$ = new AstPropSpec{$1, $8, nullptr, new AstLogOr{$1, $4, $10}};
-                          BBUNSUP($<fl>1, "Unsupported: property '(disable iff (...) @ (...)'\n"
-                                  + $<fl>1->warnMore()
-                                  + "... Suggest use property '(@(...) disable iff (...))'"); }
-        //UNSUP remove above
-        |       yDISABLE yIFF '(' expr ')' pexpr        { $$ = new AstPropSpec{$4->fileline(), nullptr, $4, $6}; }
+                yDISABLE yIFF '(' expr ')' pexpr        { $$ = new AstPropSpec{$4->fileline(), nullptr, $4, $6}; }
         |       pexpr                                   { $$ = new AstPropSpec{$1->fileline(), nullptr, nullptr, $1}; }
         ;

@@ -6913,7 +6898,21 @@ sexpr<nodeExprp>:  // ==IEEE: sequence_expr  (The name sexpr is important as reg
         |       ~p~sexpr yWITHIN sexpr
                         { $$ = new AstSWithin{$2, $1, $3}; }
         //                      // Note concurrent_assertion had duplicate rule for below
-        //UNSUP clocking_event ~p~sexpr %prec prSEQ_CLOCKING    { }
+        //                      // IEEE: clocking_event ~p~sexpr
+        //                      // expanded clocking_event
+// FIXME need production/Ast updates, perhaps similar to olD FIXME below from property_spec
+        |       '@' id ~p~sexpr %prec prSEQ_CLOCKING
+                        { $$ = new AstSenItem{$<fl>2, VEdgeType::ET_CHANGED,
+                                              new AstParseRef{$<fl>2, *$2, nullptr, nullptr}}; }
+// FIXME need production/Ast updates, perhaps similar to olD FIXME below from property_spec
+        |       '@' '(' event_expression ')' ~p~sexpr %prec prSEQ_CLOCKING
+                        { $$ = $3; }
+//FIXME-old-property_spec_rule:                '@' '(' senitem ')' yDISABLE yIFF '(' expr ')' pexpr
+//FIXME-old-property_spec_rule:                        { $$ = new AstPropSpec{$1, $3, $8, $10}; }
+//FIXME-old-property_spec_rule:        |       '@' '(' senitem ')' pexpr
+//FIXME-old-property_spec_rule:                        { $$ = new AstPropSpec{$1, $3, nullptr, $5}; }
+//FIXME-old-property_spec_rule:        |       '@' senitemVar pexpr
+//FIXME-old-property_spec_rule:                        { $$ = new AstPropSpec{$1, $2, nullptr, $3}; }
         //
         //============= expr rules copied for sequence_expr
         |       BISONPRE_COPY_ONCE(expr,{s/~l~/s/g; s/~p~/s/g; s/~noPar__IGNORE~'.'/yP_PAR__IGNORE /g; })  // {copied}