Fail to parse `nature` terminated by `;`

[guitorri]

nature [name]; --> parser fails at ;

[ngwood]

An optional semicolon is also valid after an identifier in a discipline declaration. Both first appeared in VAMS-LRM-2.3.

[ngwood]

This was actually easier to solve than I thought it would be.

diff --git a/admsXml/verilogaYacc.y.in b/admsXml/verilogaYacc.y.in
index e746d00..a8bee0c 100644
--- a/admsXml/verilogaYacc.y.in
+++ b/admsXml/verilogaYacc.y.in
@@ -140,6 +140,7 @@ R_s.admsParse
         ;
 R_discipline_member
         | tk_discipline R_discipline_name R_l.discipline_assignment tk_enddiscipline
+        | tk_discipline R_discipline_name ';' R_l.discipline_assignment tk_enddiscipline
           _ adms_admsmain_list_discipline_prepend_once_or_abort(root(),gDiscipline);
           _ gDiscipline=NULL;
         ;
@@ -175,6 +176,7 @@ R_discipline.naturename

 R_nature_member
         | tk_nature tk_ident R_l.nature_assignment tk_endnature
+        | tk_nature tk_ident ';' R_l.nature_assignment tk_endnature
           _ p_nature mynature=NULL;
           _ if(gNatureAccess) 
           _   mynature=adms_admsmain_list_nature_prepend_by_id_once_or_abort(root(),gNatureAccess);

[ngwood]

I am very embarrassed. This doesn't do what I thought it did! The code is getting assigned to the new rules, meaning the versions without the semicolons are not longer valid, which is certainly not what we want. I was able to get it to work by duplicating the code and putting it with each rule, but this is definitely not the best way of doing this. My limited knowledge of bison means I cannot suggest a better solution right now.

[felix-salfelder]

On Tue, Jan 04, 2022 at 11:05:42AM -0800, Neal wrote:

I am very embarrassed.

Don't worry about it. I'm glad you are trying at all.

This doesn't do what I thought it did! The code is getting assigned to the new rules, meaning the versions without the semicolons are not longer valid, which is certainly not what we want. I was able to get it to work by duplicating the code and putting it with each rule, but this is definitely not the best way of doing this. My limited knowledge of bison means I cannot suggest a better solution right now.

I was wondering if we can have a maybe_semicolon token that matches ';' or nothing. And then do

• | tk_discipline R_discipline_name R_l.discipline_assignment tk_enddiscipline
• | tk_discipline R_discipline_name maybe_semicolon R_l.discipline_assignment tk_enddiscipline

?

I am not a grammar expert, and I dont know what it takes to define maybe_semicolon.

Also, I did not spot the mistake. Maybe I/we/adms should require (simple end-to-end) tests before anything gets in. But this will only raise the bar for contributions. :/

[ngwood]

Success!

diff --git a/admsXml/verilogaYacc.y.in b/admsXml/verilogaYacc.y.in
index a8bee0c..fc7b7a3 100644
--- a/admsXml/verilogaYacc.y.in
+++ b/admsXml/verilogaYacc.y.in
@@ -138,9 +138,12 @@ R_s.admsParse
         | R_discipline_member
         | R_nature_member
         ;
+R_optional_semicolon
+        |
+        | ';'
+        ;
 R_discipline_member
-        | tk_discipline R_discipline_name R_l.discipline_assignment tk_enddiscipline
-        | tk_discipline R_discipline_name ';' R_l.discipline_assignment tk_enddiscipline
+        | tk_discipline R_discipline_name R_optional_semicolon R_l.discipline_assignment tk_enddiscipline
           _ adms_admsmain_list_discipline_prepend_once_or_abort(root(),gDiscipline);
           _ gDiscipline=NULL;
         ;
@@ -175,8 +178,7 @@ R_discipline.naturename
         ;

 R_nature_member
-        | tk_nature tk_ident R_l.nature_assignment tk_endnature
-        | tk_nature tk_ident ';' R_l.nature_assignment tk_endnature
+        | tk_nature tk_ident R_optional_semicolon R_l.nature_assignment tk_endnature
           _ p_nature mynature=NULL;
           _ if(gNatureAccess) 
           _   mynature=adms_admsmain_list_nature_prepend_by_id_once_or_abort(root(),gNatureAccess);

My book on Flex/Bison is still in the post! I just wanted this fixed for the next release.

I created a new rule for an optional semicolon. Then I added this in place of the previous change.

I've tested with Accellera 2.2 (no semicolons) and Accellera 2.3.1 (semicolons) disciplines.vams files and they both work.

[felix-salfelder]

On Tue, Jan 04, 2022 at 11:33:31AM -0800, Neal wrote:

Success! [..] I created a new rule for an optional semicolon. There I added this in place of the previous change.

Thanks. merged.