SystemVerilog: support property:parameter (update for #2537) #2666

hirooih · 2020-10-13T12:51:29Z

This is a fix for #2537.

By adding option --fields-SystemVerilog=+{property} ctags outputs a tag property:parameter on parameters which can be overridden on instantiation.

I choose a generic name property instead of definedWith. This is because, as we discussed on #2537, what we need is not how a constant is defined with, parameter or localparam.

hirooih · 2020-10-13T13:12:04Z

/root/universal-ctags/Tmain/list-fields.d/stdout-diff.txt

	--- /root/universal-ctags/Tmain/list-fields.d/stdout-actual.txt	2020-10-13 12:54:14.564830072 +0000
	+++ ./Tmain/list-fields.d/stdout-expected.txt	2020-10-13 12:53:33.300773578 +0000
	@@ -55,2 +54,0 @@
	--	property	no	SystemVerilog	s--	no	additional properties.  ('property:name' or 'property:name:value')
	--	property	no	Verilog	s--	no	additional properties.  ('property:name' or 'property:name:value')

/root/universal-ctags/Tmain/list-fields-with-prefix.d/stdout-diff.txt

	--- /root/universal-ctags/Tmain/list-fields-with-prefix.d/stdout-actual.txt	2020-10-13 12:54:14.912830548 +0000
	+++ ./Tmain/list-fields-with-prefix.d/stdout-expected.txt	2020-10-13 12:53:33.300773578 +0000
	@@ -37,2 +36,0 @@
	--       UCTAGSproperty       no      SystemVerilog    s--    no    additional properties.  ('property:name' or 'property:name:value')
	--       UCTAGSproperty       no      Verilog          s--    no    additional properties.  ('property:name' or 'property:name:value')

make: *** [Makefile:5821: tmain] Error 1

It seems that I have to update ./Tmain/list-fields-with-prefix.d/stdout-expected.txt.
I will update it after I have a feedback about my choise of property:parameter from @masatake san.

masatake · 2020-10-13T13:19:58Z

A negative integer K_PARAMETER is passed to createTag().

[jet@living]~/var/ctags% cat /tmp/input.sv 
parameter string i =

[jet@living]~/var/ctags% ./ctags /tmp/input.sv 
ctags: parsers/verilog.c:786: createTag: Assertion `kind >= 0' failed.
ctags: parsers/verilog.c:786: parsing /tmp/input.sv:1 as SystemVerilog
zsh: abort (core dumped)  ./ctags /tmp/input.sv
[jet@living]~/var/ctags% gdb ./ctags
GNU gdb (GDB) Fedora 9.1-6.fc32
Copyright (C) 2020 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./ctags...
warning: File "/home/jet/var/ctags/.gdbinit" auto-loading has been declined by your `auto-load safe-path' set to "$debugdir:$datadir/auto-load:/usr/lib/golang/src/runtime/runtime-gdb.py".
To enable execution of this file add
	add-auto-load-safe-path /home/jet/var/ctags/.gdbinit
line to your configuration file "/home/jet/.gdbinit".
To completely disable this security protection add
	set auto-load safe-path /
line to your configuration file "/home/jet/.gdbinit".
For more information about this security protection see the
"Auto-loading safe path" section in the GDB manual.  E.g., run from the shell:
	info "(gdb)Auto-loading safe path"
(gdb) r /tmp/input.sv 
Starting program: /home/jet/var/ctags/ctags /tmp/input.sv
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.31-4.fc32.x86_64
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
ctags: parsers/verilog.c:786: createTag: Assertion `kind >= 0' failed.
ctags: parsers/verilog.c:786: parsing /tmp/input.sv:1 as SystemVerilog

Program received signal SIGABRT, Aborted.
0x00007ffff7c3c9e5 in raise () from /lib64/libc.so.6
Missing separate debuginfos, use: dnf debuginfo-install jansson-2.12-5.fc32.x86_64 libseccomp-2.5.0-3.fc32.x86_64 libxml2-2.9.10-7.fc32.x86_64 libyaml-0.2.2-3.fc32.x86_64 xz-libs-5.2.5-1.fc32.x86_64 zlib-1.2.11-21.fc32.x86_64
(gdb) where
#0  0x00007ffff7c3c9e5 in raise () from /lib64/libc.so.6
#1  0x00007ffff7c25895 in abort () from /lib64/libc.so.6
#2  0x0000000000490361 in debugAssert (assertion=assertion@entry=0x4d1200 "kind >= 0", file=file@entry=0x4d11b0 "parsers/verilog.c", 
    line=line@entry=786, function=function@entry=0x4d1ec0 <__func__.1> "createTag") at main/debug.c:136
#3  0x0000000000482d6d in createTag (token=token@entry=0x56c160, kind=kind@entry=K_PARAMETER) at ./main/vstring.h:104
#4  0x000000000048384c in tagNameList (token=token@entry=0x56c160, c=<optimized out>) at parsers/verilog.c:1530
#5  0x000000000048446b in findTag (token=0x56c160) at parsers/verilog.c:1575
#6  findVerilogTags () at parsers/verilog.c:1670
#7  0x000000000041244b in createTagsForFile (passCount=1, language=102) at main/parse.c:3722
#8  createTagsWithFallback1 (language=language@entry=102, exclusive_subparser=exclusive_subparser@entry=0x7fffffffdadc)
    at main/parse.c:3844
#9  0x0000000000412764 in createTagsWithFallback (failureInOpenning=<synthetic pointer>, mio=0x0, language=102, 
    fileName=0x51e870 "/tmp/input.sv") at main/parse.c:3934
#10 parseMio (fileName=fileName@entry=0x51e870 "/tmp/input.sv", language=language@entry=102, mio=0x0, 
    useSourceFileTagPath=useSourceFileTagPath@entry=true, clientData=clientData@entry=0x0) at main/parse.c:4096
#11 0x00000000004128f5 in parseFileWithMio (fileName=fileName@entry=0x51e870 "/tmp/input.sv", mio=mio@entry=0x0, 
    clientData=clientData@entry=0x0) at main/parse.c:4142
#12 0x0000000000412a9a in parseFile (fileName=fileName@entry=0x51e870 "/tmp/input.sv") at main/parse.c:4079
#13 0x0000000000403cb8 in createTagsForEntry (entryName=0x51e870 "/tmp/input.sv") at main/main.c:221
#14 0x0000000000403ff0 in createTagsForArgs (args=0x51e810) at main/main.c:266
#15 batchMakeTags (args=0x51e810, user=<optimized out>) at main/main.c:360
#16 0x0000000000404457 in runMainLoop (args=0x51e810) at main/main.c:585
#17 ctags_cli_main (argc=<optimized out>, argv=<optimized out>) at main/main.c:585
#18 0x0000000000403a26 in main (argc=2, argv=0x7fffffffdd28) at main/cmd.c:21

hirooih · 2020-10-13T13:35:18Z

Thank you for your prompt reply as always.

I cannot reproduce the fail. I'm afraid that Assert() is disabled on my environment.

$ make clean
...
$ make DEBUG=1
...
$ cat >/tmp/test.sv
parameter string i =
$ cat /tmp/test.sv
parameter string i =
$ ./ctags /tmp/test.sv
$

Do you see any problem above?

hirooih · 2020-10-13T13:52:30Z

I found a bug in my code and commit the fix.
But it seems that Assert() is disabled. How can I enable it?

masatake · 2020-10-13T20:40:49Z

But it seems that Assert() is disabled. How can I enable it?

$ ./configure --enable-debugging
$ make clean;make

masatake · 2020-10-13T22:06:18Z

I read https://www.cnblogs.com/xgcl-wei/p/9090918.html with Google Translate.
Though I wrote "do as you want", I think we should introduce new kinds.

diff --git a/parsers/verilog.c b/parsers/verilog.c
index 94020bbc..49f9052a 100644
--- a/parsers/verilog.c
+++ b/parsers/verilog.c
@@ -119,19 +119,23 @@ static kindDefinition VerilogKinds [] = {
 };
 
 static kindDefinition SystemVerilogKinds [] = {
- { true, 'c', "constant",  "constants (define, parameter, specparam, enum values)" },
+ { true, 'c', "constant",  "constants (never used, placeholder)" },
+ { true, 'd', "definition","items defined with `define" },
  { true, 'e', "event",     "events" },
  { true, 'f', "function",  "functions" },
+ { true, 'l', "localparam","localparams" },
  { true, 'm', "module",    "modules" },
  { true, 'n', "net",       "net data types" },
+ { true, 'o', "parameter", "parameters (overridable)" },
  { true, 'p', "port",      "ports" },
  { true, 'r', "register",  "register data types" },
+ { true, 's', "specparam", "specparams" },
  { true, 't', "task",      "tasks" },
  { true, 'b', "block",     "blocks" },
  { true, 'A', "assert",    "assertions" },
  { true, 'C', "class",     "classes" },
  { true, 'V', "covergroup","covergroups" },
- { true, 'E', "enum",      "enumerators" },
+ { true, 'E', "enum",      "enumerators (values inside an enumeration)" },
  { true, 'I', "interface", "interfaces" },
  { true, 'M', "modport",   "modports" },
  { true, 'K', "package",   "packages" },

hirooih · 2020-10-13T23:47:03Z

I read https://www.cnblogs.com/xgcl-wei/p/9090918.html with Google Translate.

Examples in this article are old Verilog style, no_parameter_port_list type only, in my example #2537 (comment).

In the example below, you need P1, P2, P3, and P.4. You don't need L1, L3, L5, although they are defined by parameter.

This is important part.

masatake · 2020-10-14T01:48:08Z

In the example below, you need P1, P2, P3, and P.4. You don't need L1, L3, L5, although they are defined by parameter.
This is important part.

ctags should be more stupid. In most cases, it reports what it sees in the source code as-is to client tools; don't hide raw information too much.
The client tools may decide whether a language object is important or not.
ctags should provide information for helping the client tools making such a decision.

Being clever(?) is o.k. but provide high-level information as hints, and don't hide raw information too much.

My idea of the way to represent the high-level information is "parameter:overridable" or "parameter:shadowed". Just an idea.
These field will be attached to tags with "parameter" kind. "property" is not so bad. But it is better to choose a more specific one for the puropose.

Even if you disagree with me, you can merge this pull request. Having a maintainer of a parser is the highest prirority in this project.

Either way, I would like you to write man/ctags-lang-systemvereilog.7.rst.in that helps developers of client tools know how to utilize tags output of systemverilog parser.

hirooih · 2020-10-14T14:10:34Z

I think you understand that we have to define a new field to solve the issue of #2537.

Introducing new kinds does not help us at all for #2537. It is totally independent issue from #2537.
I agree with you that providing raw information is better than not. The only issue here is backward compatibility as we discussed.

"property" is not so bad. But it is better to choose a more specific one for the purpose.

This is the point I wanted to hear from you.

When we introduce a new tag-field, do we have to disable it by default as your patch did on #2537 (comment) ?
If we do, I would like to choose a generic name because I don't want to let users (including me:-)) to add new --fields-SystemVerilog=+{XXX} option every time we introduce a new tag-field.
If we can enable it by default, I would like to choose specific name as you proposed.

tags man page does not explicitly described about this, but unsupported optional tag-field should be ignored by tools reading tags file. Even if a tags tool cannot ignore unsupported tag-field, users can disable explicitly by adding --fields-SystemVerilog=-{XXX} option. Enabling new tag-field does not break backward compatibility, so it should be OK.

Do you agree with me?

masatake · 2020-10-14T15:38:31Z

When we introduce a new tag-field, do we have to disable it by default as your patch did on #2537 (comment)?

No. I disabled it because I was not sure whether I should enable it or not.
In many cases, a person who reports a bug of a parser of asks enhancement to a parser knows well about the target language but doesn't know about ctags. On the other hand, I know about ctags well but don't know about the target language.
So I can modify the code as the person wants. However, I cannot convince myself that I should merge the code.
I make the feature I don't understand disable by default. You know both ctags and SystemVerilog, so you may know whether a new feature should be enabled or not.

If we do, I would like to choose a generic name because I don't want to let users (including me:-)) to add new --fields-SystemVerilog=+{XXX} option every time we introduce a new tag-field.
If we can enable it by default, I would like to choose specific name as you proposed.

You can choose specific name with enabling the field by default.
Two notes:
(1) don't use name, input and pattern as the name of fields.
See the output of FIXED field of ctags --list-fields.
ctags prints the field always, and they don't have "field:" prefix in tags output.
so..well...I cannot remember why this rule is important. This rule is related to (2).
C++ parser violates this rule. I must write this to ctags-optlib(7) man page if I can remember the reason.

(2) the field name can conflict each other within the main part and parsers.
AutoIt parser has properties field. C and C++ also have.
If you give a source tree including both AutoIt and C source files, a you can distinguish that "properties" field in a
tag entry is part of AutoIt or C quickly. However, if you run --fields=+{language}, "language:" field may help you
for distinguishing. I must write this to ctags-client-tools(7) man page.

tags man page does not explicitly described about this, but unsupported optional tag-field should be ignored by tools reading tags file. Even if a tags tool cannot ignore unsupported tag-field, users can disable explicitly by adding --fields-SystemVerilog=-{XXX} option. Enabling new tag-field does not break backward compatibility, so it should be OK.

Yes, it is OK. Your understanding is the same as mine.

BTW, when I started working on u-ctags, I had the similar concern (not the same).
So I introduce --put-field-prefix. See man/ctags.1.rst.in.

Do you agree with me?

YES.

hirooih · 2020-10-14T23:40:04Z

Thank you. I will change the field name to specific one.
But I will stay this feature disabled because it is required very only a specific use-case.

coveralls · 2020-10-15T14:39:29Z

Coverage increased (+0.01%) to 87.027% when pulling f5f4405 on hirooih:sv-parameter-property into 663bfb1 on universal-ctags:master.

codecov · 2020-10-15T14:44:59Z

Codecov Report

Merging #2666 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2666      +/-   ##
==========================================
+ Coverage   86.92%   86.93%   +0.01%     
==========================================
  Files         183      183              
  Lines       39053    39077      +24     
==========================================
+ Hits        33946    33971      +25     
+ Misses       5107     5106       -1

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

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 663bfb1...f5f4405. Read the comment docs.

hirooih · 2020-10-15T14:53:16Z

I chose parameter:overridden as the field name.
I also fix Tmain tests for field name and add parser-verilog.rst.

Please review them.

masatake · 2020-10-15T19:11:36Z

If you will give a value other than "overridden" to the "parameter" field in future development (like "parameter:shadowed"), "parameter:overridden" is o.k.

If not, I think the field name should be "overridden"; attach "overridden:" instead of "parameter:overridden".
This is a boolean field.

The following change implements what I wrote for "If not" case.

diff --git a/parsers/verilog.c b/parsers/verilog.c
index 1fef2146..b7241cf7 100644
--- a/parsers/verilog.c
+++ b/parsers/verilog.c
@@ -308,8 +308,9 @@ static const char *systemVerilogKeywords[] = {
 };
 
 static fieldDefinition VerilogFields[] = {
-       { .name = "parameter",
+       { .name = "overridden",
          .description = "parameter whose value can be overridden.",
+         .dataType = FIELDTYPE_BOOL,
          .enabled = false },
 };
 
@@ -843,7 +844,7 @@ static void createTag (tokenInfo *const token, verilogKind kind)
 
        if (token->parameter)
                attachParserField (&tag, false,
-                                                  VerilogFields [F_PARAMETER_OVERRIDDEN].ftype, "overridden");
+                                                  VerilogFields [F_PARAMETER_OVERRIDDEN].ftype, "");
 
        int corkIndex = makeTagEntry (&tag);
        if (isInputLanguage (Lang_systemverilog)

However, this change doesn't work well. I must change the main part to support FIELDTYPE_BOOL.
I'm working on this topic at #2668. (I will merge #2668 even if you choose "parameter:overridden".)

If all the changes are merged well, the parser works as follows:

$ cat foo.sv
module m
  parameter  P;
  localparam L;
endmodule
$ ./ctags --fields-SystemVerilog=+'{overridden}' --kinds-SystemVerilog=-m -o - foo.sv
L	foo.sv	/^  localparam L;$/;"	c
P	foo.sv	/^  parameter  P;$/;"	c	overridden:
$ ./ctags --fields-SystemVerilog=+'{overridden}' --kinds-SystemVerilog=-m --output-format=json -o - foo.sv
./ctags --fields-SystemVerilog=+'{overridden}' --kinds-SystemVerilog=-m --output-format=json -o - foo.sv
{"_type": "tag", "name": "L", "path": "foo.sv", "pattern": "/^  localparam L;$/", "kind": "constant"}
{"_type": "tag", "name": "P", "path": "foo.sv", "pattern": "/^  parameter  P;$/", "kind": "constant", "overridden": true}
$ ./ctags --fields-SystemVerilog=+'{overridden}' --kinds-SystemVerilog=-m --output-format=xref --_xformat="%{name} %{SystemVerilog.overridden}" -o - foo.sv
<kinds-SystemVerilog=-m --output-format=xref --_xformat="%{name} %{SystemVerilog.overridden}" -o - foo.sv
L 
P overridden

masatake · 2020-10-15T19:20:58Z

A parser can have two types of documents; one for potential contributors who improve the parser, and users that include developers of client tools.

docs/parser-.rst is for potential contributors.
man/ctags-lang-.7.rst.in is for users.

I think your new document is for users. So it should be man/ctags-lang-verilog.7.rst.in.
I must tell the way to add a man page. However, I cannot find enough time to do so.
So could you drop the change for the document temporarily from this pull request?
After merging the changes for the code, I would like to work on the documentation with you.

hirooih · 2020-10-16T03:38:19Z

I've been thinking of a better name since I read your message this morning.

If not, I think the field name should be "overridden"; attach "overridden:" instead of "parameter:overridden".
This is a boolean field.

Yes, this field is boolean. I did not think the value part should be empty for boolean field.
overridden is too broad. I think we should include "parameter" in the field name.

How about "overridable_parameter:"?

I thought "parameter:overridden" is better than "parameter:overridable", you proposed.
Because "a parameter is overridden", but "a parameter does not override something".

"overridden_parameter:" is wrong.
But I think "overridable_parameter:" is OK, "a module instance override a parameter.".

I think your new document is for users. So it should be man/ctags-lang-verilog.7.rst.in.

I will make man/ctags-lang-verilog.7.rst.in referencing man/ctags-lang-python.7.rst.in this evening.

masatake · 2020-10-16T05:55:26Z

_ (and -) in a field name is not allowed. (see tags(5)).

Yes, this field is boolean. I did not think the value part should be empty for boolean field.
overridden is too broad. I think we should include "parameter" in the field name.

How about parameter: or parametric:?
If we introduce parameter kind, parameter: field is a bit odd. However, they are simple.

overriddenable: is the ideal word but there is no such word in English.
canBeOverridden: is an alternative.

I think we have discussed the name much. So I would like you to choose as you want.
I will clean up #2688 so that you can define the boolean field.

BTW, misc/review script may help you to update the expected files of test cases.

hirooih · 2020-10-16T15:39:35Z

I tried overridableParameter, but it was too long. It affects on the format of the fields list of all languages.

I decided to go with parameter.
I've committed the fix. Please check again, especially ctags-lang-verilog.7.rst.in.

I will clean up #2668 so that you can define the boolean field.

Is this for json-writer?

BTW, misc/review script may help you to update the expected files of test cases.

Very cool bash script! This will help me a lot.

hirooih · 2020-10-19T14:04:33Z

@masatake san,

May I commit this pull-request?

You reviewed my code already. The only my concern is ctags-lang-verilog.7.rst.in. I might have misunderstandings.

masatake · 2020-10-19T18:33:11Z

Could you wait for a while (2 or 3 days)?
I got trouble with my health.

man/ctags-lang-verilog.7.rst.in

masatake · 2020-10-19T19:23:25Z

I merged #2688. So we can define a boolean type field.
So I would like to define the "parameter:" field as a boolean.
I will put comments on your change.

parsers/verilog.c

masatake · 2020-10-19T19:42:04Z

See man/README.

Apply the following diff:

diff --git a/Makefile.am b/Makefile.am
index 799a256f..e916442d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -221,6 +221,7 @@ man_MANS = \
        man/ctags-optlib.7 \
        \
        man/ctags-lang-python.7 \
+       man/ctags-lang-verylog.7 \
        \
        $(NULL)
 rst2man_verbose = $(rst2man_verbose_@AM_V@)
diff --git a/configure.ac b/configure.ac
index 01206b78..3912a8b1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -725,6 +725,7 @@ AC_CONFIG_FILES([Makefile
                 man/ctags-incompatibilities.7.rst
                 man/ctags-optlib.7.rst
                 man/ctags-lang-python.7.rst
+                man/ctags-lang-verilog.7.rst
                 man/readtags.1.rst
                 ])
 
diff --git a/docs/man-pages.rst b/docs/man-pages.rst
index 0d34f8e8..2885dba6 100644
--- a/docs/man-pages.rst
+++ b/docs/man-pages.rst
@@ -11,6 +11,7 @@ Man pages
        ctags-client-tools(7) <man/ctags-client-tools.7.rst>
        ctags-incompatibilities(7) <man/ctags-incompatibilities.7.rst>
        ctags-lang-python(7) <man/ctags-lang-python.7.rst>
+       ctags-lang-verilog(7) <man/ctags-lang-verilog.7.rst>
        ctags-optlib(7) <man/ctags-optlib.7.rst>
        readtags(1) <man/readtags.1.rst>
        tags(5) <man/tags.5.rst>
diff --git a/man/Makefile b/man/Makefile
index 3a532b5a..d9606812 100644
--- a/man/Makefile
+++ b/man/Makefile
@@ -48,6 +48,7 @@ IN_SOURCE_FILES = \
        ctags-client-tools.7.rst.in \
        \
        ctags-lang-python.7.rst.in \
+       ctags-lang-verilog.7.rst.in \
        \
        readtags.1.rst.in \
        \

generate docs/man/ctags-lang-verilog.7.rst with running make QUICK=1 update-docs at the man directory
do git add docs/man/ctags-lang-verilog.7.rst && git commit --amend

So users can read your man page at https://docs.ctags.io/en/latest/man-pages.html

masatake · 2020-10-19T19:44:52Z

I will clean up #2668 so that you can define the boolean field.

Is this for json-writer?

Yes. The change about boolean is done in my pull request. So you don've have to do it on your side.

hirooih · 2020-10-19T23:33:01Z

Thank you for your comments.

Could you wait for a while (2 or 3 days)?
I got trouble with my health.

I can wait.
I'm sorry it seems I rushed you. Please take care of yourself.

Implements LRM 6.20.1 rule - If the declaration of a design element uses a parameter_port_list (even an empty one), then in any parameter_declaration directly contained within the declaration, the parameter keyword shall be a synonym for the localparam keyword (see 6.20.4). - All param_assignments appearing within a class body shall become localparam declarations regardless of the presence or absence of a parameter_port_list. - All param_assignments appearing within a generate block, package, or compilation-unit scope shall become localparam declarations. Only "generate block" rule is not implemented yet, because it does not create a context.

hirooih · 2020-10-21T14:02:20Z

First please take a rest enough until you feel better.

I've commit the fix.
It is almost as you suggested.
The only change I had to is adding a variable fieldTable which holds VerilogFields or SystemVerilogFields.

Whoops! The regression tests failed.

/root/universal-ctags/Tmain/list-fields.d/stdout-diff.txt

	--- /root/universal-ctags/Tmain/list-fields.d/stdout-actual.txt	2020-10-21 13:50:36.031687955 +0000
	+++ ./Tmain/list-fields.d/stdout-expected.txt	2020-10-21 13:49:57.227650070 +0000
	@@ -55,2 +55,2 @@
	--	parameter	no	SystemVerilog	--b	no	parameter whose value can be overridden.
	--	parameter	no	Verilog	--b	no	parameter whose value can be overridden.
	+-	parameter	no	SystemVerilog	s--	no	parameter whose value can be overridden.
	+-	parameter	no	Verilog	s--	no	parameter whose value can be overridden.

/root/universal-ctags/Tmain/list-fields-with-prefix.d/stdout-diff.txt

	--- /root/universal-ctags/Tmain/list-fields-with-prefix.d/stdout-actual.txt	2020-10-21 13:50:36.239688145 +0000
	+++ ./Tmain/list-fields-with-prefix.d/stdout-expected.txt	2020-10-21 13:49:57.227650070 +0000
	@@ -37,2 +37,2 @@
	--       UCTAGSparameter      no      SystemVerilog    --b    no    parameter whose value can be overridden.
	--       UCTAGSparameter      no      Verilog          --b    no    parameter whose value can be overridden.
	+-       UCTAGSparameter      no      SystemVerilog    s--    no    parameter whose value can be overridden.
	+-       UCTAGSparameter      no      Verilog          s--    no    parameter whose value can be overridden

I have no idea about this. I guess this is from the fix of #2668. Are they expected changes?

Again I don't want rush you. Please take a look this after you gets well.

masatake · 2020-10-21T20:42:22Z

Again I don't want rush you. Please take a look this after you gets well.

Thank you. I'm feeling better now.

I have no idea about this. I guess this is from the fix of #2668. Are they expected changes?

Yes, they are expected changes. Please update stdout-expected.txt.
s-- means "string". --b means "boolean". See the description of JSTYPE of --list-fields in man/ctags.1.rst.in for more details.

masatake

Except the stdout-expected.txt, the changes look good for me.
I think it is better to enable the new field by default because the information brought by the field is so useful. However, whether it should be enabled or not is up-to the parser maitainer.

…stemVerilog

hirooih · 2020-10-22T13:51:29Z

I'm feeling better now.

Good!

I think it is better to enable the new field by default because the information brought by the field is so useful.

Let me stay this feature disabled by default. Because it is required by only a very specific use-case.

However, whether it should be enabled or not is up-to the parser maintainer.

Except the stdout-expected.txt, the changes look good for me.

All tests passed.
I've merged this pull request.

hirooih · 2020-10-24T08:47:33Z

@masatake san,

I found I had to add man/ctags-lang-verilog.7 and man/ctags-lang-verilog.7.rst to .gitignore.
How should I add? Shall I open a new pull-request? Or is there better way?

masatake · 2020-10-24T10:24:59Z

Please, open a new pull-request, and merge it. In such a simple one, you don't need my review but I would like you to make a pull reqeust for it.

It seems that we use "gitignore:" as prefix.

commit 95854ef9a7087ca6b4181416ef227736067925b8
Author: Pierce Lopez <pierce.lopez@gmail.com>
Date:   Mon Sep 14 00:32:11 2020 -0400

    gitignore: add ctags-lang-python generated manpage files

hirooih mentioned this pull request Oct 13, 2020

(System-)Verilog: Parameters/Localparams/Specparams should be seperated #2537

Closed

hirooih force-pushed the sv-parameter-property branch from c22d7f6 to e793821 Compare October 13, 2020 13:44

hirooih force-pushed the sv-parameter-property branch from e793821 to 0b94697 Compare October 15, 2020 14:35

hirooih force-pushed the sv-parameter-property branch from 0b94697 to eabd96a Compare October 16, 2020 15:27

masatake reviewed Oct 19, 2020

View reviewed changes

man/ctags-lang-verilog.7.rst.in Show resolved Hide resolved

masatake requested changes Oct 19, 2020

View reviewed changes

hirooih added 3 commits October 20, 2020 23:00

SystemVerilog: parameter property support (part 1)

2e2a1af

Units,SystemVerilog: add parameter tests for universal-ctags#2537

a7f299b

hirooih force-pushed the sv-parameter-property branch from eabd96a to 74fcee6 Compare October 21, 2020 13:47

masatake approved these changes Oct 21, 2020

View reviewed changes

Tmain,SystemVerilog: introduce a new field "parameter" for Verilog/Sy…

b515a27

…stemVerilog

hirooih force-pushed the sv-parameter-property branch from 74fcee6 to 97cbd5b Compare October 22, 2020 12:24

docs(web),SystemVerilog: add ctags-lang-verilog.7.rst.in

f5f4405

hirooih force-pushed the sv-parameter-property branch from 8cecf95 to f5f4405 Compare October 22, 2020 13:01

hirooih merged commit 68d0478 into universal-ctags:master Oct 22, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SystemVerilog: support property:parameter (update for #2537) #2666

SystemVerilog: support property:parameter (update for #2537) #2666

hirooih commented Oct 13, 2020

hirooih commented Oct 13, 2020

masatake commented Oct 13, 2020

hirooih commented Oct 13, 2020

hirooih commented Oct 13, 2020

masatake commented Oct 13, 2020

masatake commented Oct 13, 2020

hirooih commented Oct 13, 2020

masatake commented Oct 14, 2020

hirooih commented Oct 14, 2020

masatake commented Oct 14, 2020

hirooih commented Oct 14, 2020

coveralls commented Oct 15, 2020 •

edited

Loading

codecov bot commented Oct 15, 2020 •

edited

Loading

hirooih commented Oct 15, 2020

masatake commented Oct 15, 2020

masatake commented Oct 15, 2020

hirooih commented Oct 16, 2020

masatake commented Oct 16, 2020

hirooih commented Oct 16, 2020 •

edited

Loading

hirooih commented Oct 19, 2020

masatake commented Oct 19, 2020

masatake commented Oct 19, 2020

masatake commented Oct 19, 2020

masatake commented Oct 19, 2020

hirooih commented Oct 19, 2020

hirooih commented Oct 21, 2020

masatake commented Oct 21, 2020

masatake left a comment

hirooih commented Oct 22, 2020

hirooih commented Oct 24, 2020

masatake commented Oct 24, 2020

SystemVerilog: support property:parameter (update for #2537) #2666

SystemVerilog: support property:parameter (update for #2537) #2666

Conversation

hirooih commented Oct 13, 2020

hirooih commented Oct 13, 2020

masatake commented Oct 13, 2020

hirooih commented Oct 13, 2020

hirooih commented Oct 13, 2020

masatake commented Oct 13, 2020

masatake commented Oct 13, 2020

hirooih commented Oct 13, 2020

masatake commented Oct 14, 2020

hirooih commented Oct 14, 2020

masatake commented Oct 14, 2020

hirooih commented Oct 14, 2020

coveralls commented Oct 15, 2020 • edited Loading

codecov bot commented Oct 15, 2020 • edited Loading

Codecov Report

hirooih commented Oct 15, 2020

masatake commented Oct 15, 2020

masatake commented Oct 15, 2020

hirooih commented Oct 16, 2020

masatake commented Oct 16, 2020

hirooih commented Oct 16, 2020 • edited Loading

hirooih commented Oct 19, 2020

masatake commented Oct 19, 2020

masatake commented Oct 19, 2020

masatake commented Oct 19, 2020

masatake commented Oct 19, 2020

hirooih commented Oct 19, 2020

hirooih commented Oct 21, 2020

masatake commented Oct 21, 2020

masatake left a comment

Choose a reason for hiding this comment

hirooih commented Oct 22, 2020

hirooih commented Oct 24, 2020

masatake commented Oct 24, 2020

coveralls commented Oct 15, 2020 •

edited

Loading

codecov bot commented Oct 15, 2020 •

edited

Loading

hirooih commented Oct 16, 2020 •

edited

Loading