Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SystemVerilog: support struct and union member #2701

Merged
merged 4 commits into from
Nov 20, 2020

Conversation

hirooih
Copy link
Contributor

@hirooih hirooih commented Nov 17, 2020

Struct and union members are emitted by this fix.
The kind "member" (w) is added. (m and M were already used.)

Enum items are emitted in the defined order with --sort=no option.

@codecov
Copy link

codecov bot commented Nov 17, 2020

Codecov Report

Merging #2701 (3173073) into master (ea5214b) will increase coverage by 0.00%.
The diff coverage is 95.65%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2701   +/-   ##
=======================================
  Coverage   86.99%   86.99%           
=======================================
  Files         185      185           
  Lines       39441    39466   +25     
=======================================
+ Hits        34310    34333   +23     
- Misses       5131     5133    +2     
Impacted Files Coverage Δ
parsers/verilog.c 98.59% <95.65%> (-0.27%) ⬇️

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 ea5214b...3173073. Read the comment docs.

@coveralls
Copy link

coveralls commented Nov 17, 2020

Coverage Status

Coverage increased (+0.003%) to 87.082% when pulling 3173073 on hirooih:sv-struct-member into e37e2ef on universal-ctags:master.

@masatake
Copy link
Member

...I don't like this change and the original code.
See the field used in popToken and pushToken. These functions use scope field.
These functions are originally written for tracking scope information.
See the comment for scope:

	struct sTokenInfo*  scope;         /* context of keyword */

However, the change and the original code abuse them for dealing with a sequence.
I think this is a bad idea.

Do you know ptrArray (main/ptrarray.h) ?

You can use ptrArray to store the sequence of tokens.
#2702 illustrates how to use ptrArray.

@hirooih
Copy link
Contributor Author

hirooih commented Nov 17, 2020

...I don't like this change and the original code.

I don't like it, either. But I also don't like to reinventing the wheels.
I reused the code which was already in verilog.c.

Do you know ptrArray (main/ptrarray.h) ?

No, I did not.
AFAIK we don't have a list of API which a parser can use. Can we use any external functions defined in main/*.h?

I have a question related to this topic.
Is there API to get the filename which a parse is parsing?

I will update verilog.c to use ptrArray.

@masatake
Copy link
Member

AFAIK we don't have a list of API which a parser can use. Can we use any external functions defined in main/*.h?

No, we don't have.
We have one rule; we should not use functions and types declared in main/*_p.h in parsers. _p means "private in main."\

I have a question related to this topic.
Is there API to get the filename which a parse is parsing?

Yes, you can use getInputFileName() declared in main/read.h.

I will update verilog.c to use ptrArray.

I would like you to see #2702 and merge it if it is o.k.

@hirooih
Copy link
Contributor Author

hirooih commented Nov 18, 2020

We have one rule; we should not use functions and types declared in main/*_p.h in parsers. _p means "private in main."

I see. This means we do have a list of API.

Yes, you can use getInputFileName() declared in main/read.h.

Thanks.

I would like you to see #2702 and merge it if it is o.k.

You raised the bar! It will cause conflicts.
It will be my first experience of review on GitHub.

@hirooih
Copy link
Contributor Author

hirooih commented Nov 19, 2020

@masatake san,

How about this? Please review again.
Thank you.

parsers/verilog.c Outdated Show resolved Hide resolved
Units updates
- systemverilog-struct.d: add complex struct and enum test
- *.d/expected.tags: update for struct and union member support
- systemverilog-package.d/input.sv: fix an unmatched label name
@hirooih hirooih merged commit 5a13631 into universal-ctags:master Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants