SystemVerilog: let tagNameList() skip paren on RHS #2644

hirooih · 2020-09-13T05:36:05Z

This fixes one of issues on #2618

  // src/tlm2/uvm_tlm2_generic_payload.svh:494
  string msg = $sformatf("GP miscompare between '%s' and '%s':\nlhs = %s\nrhs = %s",
      bar(), 1, 2, 3);  // register:bar => not defined

verilog.c: L1275

				} while (c != EOF && c != ','  &&  c != ';' && c != ')');

tagNameList() looks for ,, ;, or ) as the end of an expression of the right-hand-side (RHS) of an assignment.
) of bar() is detected as the end of an expression. This confuses the parser.

It is fixed by calling skipPastMatch ("()").

I also added testcases for assignment pattern.
User defined type variables are not emitted due to #2413 issue. When #2413 is fixed, they will be emitted. expected.tags must be updated at that time.

(I tried rebase -i but failed. Commit logs for merged commits remains. Sorry.)

coveralls · 2020-09-13T05:40:46Z

Coverage increased (+0.0006%) to 86.925% when pulling 3b8b5bd on hirooih:sv-tagNameList-paren-on-RHS into c88becd on universal-ctags:master.

masatake · 2020-09-13T08:30:32Z

I will arrange the commits and push the result to your branch with "git push --force".

hirooih · 2020-09-13T08:44:38Z

I did "git push --force" before sending pull-request.

I did now again but same result.

$ git pull --force
Already up to date.
$

I probably I made a mistake.
I might do git merge upstream/master on this branch (but not sure).
I should do it on my master branch and do rebase master on this branch.

Any clue?

masatake · 2020-09-13T08:52:51Z

I rearranged your commits in the branch.
I found 3 new commits:

The first thing that I want you to do is verifying the 3 commits and make sure they are the commits you want me to pull.
This is an important step because the next steps can break your commits in your local PC.

codecov · 2020-09-13T08:56:03Z

Codecov Report

Merging #2644 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2644   +/-   ##
=======================================
  Coverage   86.82%   86.82%           
=======================================
  Files         183      183           
  Lines       38971    38973    +2     
=======================================
+ Hits        33837    33839    +2     
  Misses       5134     5134

Impacted Files	Coverage Δ
parsers/verilog.c	`97.21% <100.00%> (+0.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b599093...3b8b5bd. Read the comment docs.

masatake · 2020-09-13T09:05:39Z

After verifying, you can go to the following steps for pulling the rearranging I made here to your local PC (your workspace):

$ git checkout master
$ git pull upstream master:master
$ git checkout -b sv-tagNameList-paren-on-RHS-rebased
$ git pull origin sv-tagNameList-paren-on-RHS:sv-tagNameList-paren-on-RHS-rebased

Here
upstream is defined in .git/config as

[remote "upstream"]
        url = https://github.com/universal-ctags/ctags.git
        fetch = +refs/heads/*:refs/remotes/upstream/*

I guess origin is already defined in .git/config.

sv-tagNameList-paren-on-RHS-rebased is the new workspace for imporving the commits.

After improving the commits, you can reflect the result to this pull request with following command:

git push origin --force sv-tagNameList-paren-on-RHS-rebased:sv-tagNameList-paren-on-RHS

masatake · 2020-09-13T09:09:52Z

BTW, don't you should not put your local change to your master branch.
As far as I know, you should use the master branch for pulling the changes made in the master branch of the upstream repository.
If your changes are merged to the upstream repository, "git pull upstream master:master" is the way for reflecting your changes to your master branch (of your local PC).

masatake · 2020-09-13T09:17:16Z

If your side is ready, let me know. I will start my review.

However, taking time for understanding git operations is not a bad idea.
The knowledge of git operations helps you keep commits clean, and helps you manage more branches.

This fixes one of issues on universal-ctags#2618 // src/tlm2/uvm_tlm2_generic_payload.svh:494 string msg = $sformatf("GP miscompare between '%s' and '%s':\nlhs = %s\nrhs = %s", bar(), 1, 2, 3); // register:bar => not defined verilog.c: L1275 } while (c != EOF && c != ',' && c != ';' && c != ')'); tagNameList() looks for ,, ;, or ) as the end of an expression of the right-hand-side (RHS) of an assignment. ) of bar() is detected as the end of an expression. This confuses the parser. It is fixed by calling skipPastMatch ("()").

hirooih · 2020-09-13T09:34:38Z

My Master,

I've update the commit log. It works great!
Thank you for your teaching me!

masatake · 2020-09-13T11:39:12Z

Units/parser-verilog.r/systemverilog-assignment.d/input.sv

+  struct { int x; int y; } s4 = '{x:2, y:3+k};
+
+  st s5 = '{default:2};
+  ab abkey[1:0] = '{'{a:1, b:1.0}, '{int:2, shortreal:2.0}};


ab is not typedef'ed. It this o.k.?

If possible, make a test input valid as SystemVerilog input.

hirooih · 2020-09-13T14:05:07Z

Sorry I made mistake on the definition of y. 2c49261
If you are OK for the fixes, I will squash 3 commits for Units test.

masatake · 2020-09-13T21:34:00Z

This fixes one of issues on #2618

  // src/tlm2/uvm_tlm2_generic_payload.svh:494
  string msg = $sformatf("GP miscompare between '%s' and '%s':\nlhs = %s\nrhs = %s",
      bar(), 1, 2, 3);  // register:bar => not defined

verilog.c: L1275

				} while (c != EOF && c != ','  &&  c != ';' && c != ')');

tagNameList() looks for ,, ;, or ) as the end of an expression of the right-hand-side (RHS) of an assignment.
) of bar() is detected as the end of an expression. This confuses the parser.

It is fixed by calling skipPastMatch ("()").

This comment may be helpful for people who does 'git log parsers/verilog.c'.
So put this to the commit log of 7db1ab6.

User defined type variables are not emitted due to #2413 issue. When #2413 is fixed, they will be emitted. expected.tags must be updated at that time.

You can put this to Units/parser-verilog.r/systemverilog-assignment.d/README (or anywhere in the commits.)

IF you have the original source of input.sv, write it to input.sv as a comment.

ctags/Units/parser-protobuf.r/syntax-proto3.d/input.proto

Line 1 in b599093

    
           // Taken from https://developers.google.com/protocol-buffers/docs/reference/proto3-spec

is an example.

As you wrote, please squash the commits into 1, 2 or 3 commits when ready for merging.

User defined type variables are not emitted due to #2413 issue. When #2413 is fixed, they will be emitted. expected.tags must be updated at that time.

Will you try to fix #2413? I expect your answer is yes:-)

hirooih · 2020-09-13T23:54:12Z

You can put this to Units/parser-verilog.r/systemverilog-assignment.d/README (or anywhere in the commits.)

It's already on the head of input.sv.

  // user defined types are ignored.
  //   https://github.com/universal-ctags/ctags/issues/2413
  // Update expected.tags when #2413 is fixed.

IF you have the original source of input.sv, write it to input.sv as a comment.

It's already in comments in input.sv.
LRM stands for Language Reference Manual. It is a very common acronym in Verilog world.
https://ieeexplore.ieee.org/document/8299595. You can download it with no charge. I recommend you to get it.

As you wrote, please squash the commits into 1, 2 or 3 commits when ready for merging.

I did it now.

Will you try to fix #2413? I expect your answer is yes:-)

It is a next topic. I am studying it now. Let's discuss on #2413.

masatake · 2020-09-14T00:24:05Z

As you wrote, three commits for a test case can be squashed into one.

hirooih · 2020-09-14T14:09:57Z

As you wrote, three commits for a test case can be squashed into one.

Sorry, this morning I typed wrong password on git push.

masatake · 2020-09-14T19:39:40Z

Thank you.

hirooih mentioned this pull request Sep 13, 2020

SystemVerilog: wrong output on LRM and UVM-1.2 code #2618

Closed

masatake force-pushed the sv-tagNameList-paren-on-RHS branch from 28e8e1f to 85348d9 Compare September 13, 2020 08:47

hirooih force-pushed the sv-tagNameList-paren-on-RHS branch from 85348d9 to 522d56e Compare September 13, 2020 09:27

masatake reviewed Sep 13, 2020

View reviewed changes

Units: add parser-verilog.r/systemverilog-assignment.d

3b8b5bd

hirooih force-pushed the sv-tagNameList-paren-on-RHS branch from 2c49261 to 3b8b5bd Compare September 14, 2020 14:06

masatake approved these changes Sep 14, 2020

View reviewed changes

masatake merged commit dc02aee into universal-ctags:master Sep 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SystemVerilog: let tagNameList() skip paren on RHS #2644

SystemVerilog: let tagNameList() skip paren on RHS #2644

hirooih commented Sep 13, 2020

coveralls commented Sep 13, 2020 •

edited

Loading

masatake commented Sep 13, 2020

hirooih commented Sep 13, 2020

masatake commented Sep 13, 2020 •

edited

Loading

codecov bot commented Sep 13, 2020 •

edited

Loading

masatake commented Sep 13, 2020

masatake commented Sep 13, 2020

masatake commented Sep 13, 2020

hirooih commented Sep 13, 2020

masatake Sep 13, 2020

hirooih commented Sep 13, 2020

masatake commented Sep 13, 2020

hirooih commented Sep 13, 2020

masatake commented Sep 14, 2020

hirooih commented Sep 14, 2020

masatake commented Sep 14, 2020

SystemVerilog: let tagNameList() skip paren on RHS #2644

SystemVerilog: let tagNameList() skip paren on RHS #2644

Conversation

hirooih commented Sep 13, 2020

coveralls commented Sep 13, 2020 • edited Loading

masatake commented Sep 13, 2020

hirooih commented Sep 13, 2020

masatake commented Sep 13, 2020 • edited Loading

codecov bot commented Sep 13, 2020 • edited Loading

Codecov Report

masatake commented Sep 13, 2020

masatake commented Sep 13, 2020

masatake commented Sep 13, 2020

hirooih commented Sep 13, 2020

masatake Sep 13, 2020

Choose a reason for hiding this comment

hirooih commented Sep 13, 2020

masatake commented Sep 13, 2020

hirooih commented Sep 13, 2020

masatake commented Sep 14, 2020

hirooih commented Sep 14, 2020

masatake commented Sep 14, 2020

coveralls commented Sep 13, 2020 •

edited

Loading

masatake commented Sep 13, 2020 •

edited

Loading

codecov bot commented Sep 13, 2020 •

edited

Loading