ctags icon indicating copy to clipboard operation
ctags copied to clipboard

Verilog coverpoint bug #3457

Open hirooih opened this issue 3 years ago • 2 comments

This is the fix for #3457.

The parser did not care of '.' in a identifier.

    coverpoint intf.x {

hirooih avatar Aug 13 '22 09:08 hirooih

Codecov Report

Merging #3458 (0c4ccff) into master (d14f055) will not change coverage. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3458   +/-   ##
=======================================
  Coverage   83.32%   83.32%           
=======================================
  Files         219      219           
  Lines       52745    52745           
=======================================
  Hits        43949    43949           
  Misses       8796     8796           
Impacted Files Coverage Δ
parsers/verilog.c 98.14% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov[bot] avatar Aug 13 '22 09:08 codecov[bot]

Could you rebase this branch on the latest master? With rebasing, we can eliminate multiple Merge remote-tracking branch 'upstream/master' commits from this pull request.

masatake avatar Aug 13 '22 17:08 masatake

@masatake san,

Could you rebase this branch on the latest master?

I need your help, again.

The followings are what I did yesterday.

Sync master with upstream.


$ git remote -v
origin  https://github.com/hirooih/ctags.git (fetch)
origin  https://github.com/hirooih/ctags.git (push)
upstream        https://github.com/universal-ctags/ctags.git (fetch)
upstream        https://github.com/universal-ctags/ctags.git (push)
$ git fetch upstream
$ git checkout master
$ git merge upstream/master

Create new branch and push.

$ git checkout -b verilog-coverpoint-bug-#3457
$ edit and commit...
$ git push origin verilog-coverpoint-bug-#3457

And send a PR on GitHub.


I am using a different PC than one I used to use before (physically same PC, but I am using a native Linux by dual boot). This may be the root of this issue. By reading your message I thought origin/master was not updated.

So I pushed master and did force-push a new branch.

$ git checkout master
$ git push origin master
$ git checkout verilog-coverpoint-bug-#3457
$ git push -f origin verilog-coverpoint-bug-#3457

But nothing happened. This is the output of git-log on verilog-coverpoint-bug-#3457.

$ git log
commit 7b1c51d65c774101fe34ad44f2f4773b6b80cf78 (HEAD -> verilog-coverpoint-bug-#3457, origin/verilog-coverpoint-bug-#3457)
Author: Hiroo HAYASHI <[email protected]>
Date:   Sat Aug 13 15:08:26 2022 +0900

    Verilog: take care of '.' in an identifier (#3457)
    
    A test case is included.

commit a3bd49d4279a86ef7e7f464973fdff89746f1ef2 (origin/master, origin/HEAD, master)
Merge: f3f82f2d d14f055e
Author: Hiroo HAYASHI <[email protected]>
Date:   Sat Aug 13 09:48:05 2022 +0900

    Merge remote-tracking branch 'upstream/master'

commit d14f055e3cdb47fea4b723f1cce07cf74c488f5d (tag: p5.9.20220807.0, upstream/master)
Merge: 55e668a3 8884e2b6
Author: Masatake YAMATO <[email protected]>
Date:   Tue Aug 2 12:20:29 2022 +0900

    Merge pull request #3453 from masatake/gperf-parser
    
    GPerf: new parser
...

With rebasing, we can eliminate multiple Merge remote-tracking branch 'upstream/master' commits from this pull request.

To do this I have to push origin/master to upstream. But it needs another PR which we don't want. I must be wrong somewhere.

hirooih avatar Aug 14 '22 02:08 hirooih

Keeping local master clean is my principle.

# git fetch upstream
# git checkout master
# git reset --hard upstream/master
# : Now your master becomes clean.
# git checkout -b verilog-coverpoint-bug-#3457-v2
# : Make a clean branch for this pull request
# git cherry-pick 7b1c51d65c774101fe34ad44f2f4773b6b80cf78
# git checkout verilog-coverpoint-bug-#3457
# git branch -m verilog-coverpoint-bug-#3457-just-for-backup
# git checkout verilog-coverpoint-bug-#3457-v2
# git branch -m verilog-coverpoint-bug-#3457
# git push --force origin verilog-coverpoint-bug-#3457:verilog-coverpoint-bug-#3457

These operations may not be optimal. But they work fine for me for years.

You did:

git push origin master

As far as I know, this operation has no impact on this pull request. See my origin/master. https://github.com/masatake/ctags is stopped at Feb 29, 2020.

masatake avatar Aug 14 '22 04:08 masatake

@masatake san,

Thank you for your advice.

As far as I know, this operation has no impact on this pull request. See my origin/master. https://github.com/masatake/ctags is stopped at Feb 29, 2020.

You taught me this before. I did not update origin/master after that.

Here is the current tree.

$ git log --oneline --decorate --graph --all
* 3cbf6a6a (origin/verilog-comment-in-multi-line-macro, verilog-comment-in-multi-line-macro) Verilog: refactor skipToNewLine()
* 801f3cbf Verilog: fix for multi-line macro with comment line
| * 5e5c5766 (origin/verilog-VERBOSE-macro, verilog-VERBOSE-macro) Verilog: introduce VERBOSE macro for internal errors
|/  
| * 7b1c51d6 (origin/verilog-coverpoint-bug-#3457, verilog-coverpoint-bug-#3457) Verilog: take care of '.' in an identifier (#3457)
|/  
*   a3bd49d4 (HEAD -> master, origin/master, origin/HEAD) Merge remote-tracking branch 'upstream/master'
|\  
| *   d14f055e (tag: p5.9.20220814.0, tag: p5.9.20220807.0, upstream/master) Merge pull request #3453 from masatake/gperf-parser
| |\  
| | * 8884e2b6 GPerf: new parser
| | * 57ac80ef docs(web): fix a broken link
... not merged for a long time...

When I changed my PC, I cloned the very old origin/master and I merged it with upstream/master. As a result a3bd49d4 was included in my PR.

# git fetch upstream
# git checkout master
# git reset --hard upstream/master

I know and am using git reset HEAD~ often, but I did not know git reset --hard upstream/master. This is the command I should use. By using it:

* 3cbf6a6a (origin/verilog-comment-in-multi-line-macro, verilog-comment-in-multi-line-macro) Verilog: refactor skipToNewLine()
* 801f3cbf Verilog: fix for multi-line macro with comment line
| * 5e5c5766 (origin/verilog-VERBOSE-macro, verilog-VERBOSE-macro) Verilog: introduce VERBOSE macro for internal errors
|/  
| * 7b1c51d6 (origin/verilog-coverpoint-bug-#3457, verilog-coverpoint-bug-#3457) Verilog: take care of '.' in an identifier (#3457)
|/  
*   a3bd49d4 (origin/master, origin/HEAD) Merge remote-tracking branch 'upstream/master'
|\  
| *   d14f055e (HEAD -> master, tag: p5.9.20220814.0, tag: p5.9.20220807.0, upstream/master) Merge pull request #3453 from masatake/gperf-parser
| |\  
| | * 8884e2b6 GPerf: new parser
| | * 57ac80ef docs(web): fix a broken link

Now HEAD->master points the same point as upstream/master does as you wrote:

# : Now your master becomes clean.

# git checkout -b verilog-coverpoint-bug-#3457-v2
# : Make a clean branch for this pull request
# git cherry-pick 7b1c51d65c774101fe34ad44f2f4773b6b80cf78
# git checkout verilog-coverpoint-bug-#3457
# git branch -m verilog-coverpoint-bug-#3457-just-for-backup
# git checkout verilog-coverpoint-bug-#3457-v2
# git branch -m verilog-coverpoint-bug-#3457
# git push --force origin verilog-coverpoint-bug-#3457:verilog-coverpoint-bug-#3457

First I thought I could simply use the rebase command.

git checkout verilog-coverpoint-bug-#3457
rebase master

But it did not work. Because verilog.c was updated so many times after the last merge that it caused lots of merge-conflicts during the rebase. I aborted it. The followings worked. (I could be brave, because I had backup in origin.)

git checkout master
git rebase --onto master origin/master origin/verilog-coverpoint-bug-#3457
git branch -d verilog-coverpoint-bug-#3457
git branch verilog-coverpoint-bug-#3457

rebase --onto moves HEAD to the point rebased. I deleted the old branch pointer and added it on the current point.

* 0349a4fd (HEAD, verilog-VERBOSE-macro) Verilog: introduce VERBOSE macro for internal errors
| * fd97cd0f (verilog-comment-in-multi-line-macro) Verilog: refactor skipToNewLine()
| * cd86b317 Verilog: fix for multi-line macro with comment line
|/  
| * 0c4ccff1 (verilog-coverpoint-bug-#3457) Verilog: take care of '.' in an identifier (#3457)
|/  
| * 3cbf6a6a (origin/verilog-comment-in-multi-line-macro) Verilog: refactor skipToNewLine()
| * 801f3cbf Verilog: fix for multi-line macro with comment line
| | * 5e5c5766 (origin/verilog-VERBOSE-macro) Verilog: introduce VERBOSE macro for internal errors
| |/  
| | * 7b1c51d6 (origin/verilog-coverpoint-bug-#3457) Verilog: take care of '.' in an identifier (#3457)
| |/  
| *   a3bd49d4 (origin/master, origin/HEAD) Merge remote-tracking branch 'upstream/master'
| |\  
| |/  
|/|   
* |   d14f055e (tag: p5.9.20220814.0, tag: p5.9.20220807.0, upstream/master, master) Merge pull request #3453 from masatake/gperf-parser

Now new branches is connected to master and upstream/master. And after force-push:

* 0349a4fd (origin/verilog-VERBOSE-macro, verilog-VERBOSE-macro) Verilog: introduce VERBOSE macro for internal errors
| * fd97cd0f (origin/verilog-comment-in-multi-line-macro, verilog-comment-in-multi-line-macro) Verilog: refactor skipToNewLine()
| * cd86b317 Verilog: fix for multi-line macro with comment line
|/  
| * 0c4ccff1 (HEAD -> verilog-coverpoint-bug-#3457, origin/verilog-coverpoint-bug-#3457) Verilog: take care of '.' in an identifier (#3457)
|/  
*   d14f055e (tag: p5.9.20220814.0, tag: p5.9.20220807.0, upstream/master, origin/master, origin/HEAD, master) Merge pull request #3453 from masatake/gperf-parser

I've learned a lot this time, too. Thank you very much.

hirooih avatar Aug 14 '22 09:08 hirooih

Thanks!

hirooih avatar Aug 14 '22 11:08 hirooih