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

CMake: implement scope and end fields for functions and macros #1

Closed

Conversation

masatake
Copy link

end: field is implemented. Could you consider merge this change to your pull request IF there is no critical performance impact?

I'm not sure whether function (and/or macro) can be nested.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@hadrielk
Copy link
Owner

I'm not sure whether function (and/or macro) can be nested.

Yup, they can.

Unfortunately your changes almost double the time it takes:

[hadrielk@Hadriels-MBP i95 (hadriel/I95-15000-more-ctags-changes)]$ ctags -f cmake.tags --options=cmake.ctags  --totals=yes -L cmake.files
349 files, 27601 lines (999 kB) scanned in 8.8 seconds (114 kB/s)
1305 tags added to tag file
1305 tags sorted in 0.00 seconds
[hadrielk@Hadriels-MBP i95 (hadriel/I95-15000-more-ctags-changes)]$ ctags -f cmake.tags --options=masatake_cmake.ctags  --totals=yes -L cmake.files
349 files, 27601 lines (999 kB) scanned in 15.7 seconds (63 kB/s)
1305 tags added to tag file
1305 tags sorted in 0.00 seconds

Let me work on it a bit to see if I can improve it.

@hadrielk
Copy link
Owner

I can get it down to about only a 10% impact on previous performance:

[hadrielk@Hadriels-MBP i95 (hadriel/I95-15000-more-ctags-changes)]$ ctags -f cmake.tags --options=cmake.ctags  --totals=yes -L cmake.files
349 files, 27601 lines (999 kB) scanned in 8.8 seconds (114 kB/s)
1305 tags added to tag file
1305 tags sorted in 0.00 seconds
[hadrielk@Hadriels-MBP i95 (hadriel/I95-15000-more-ctags-changes)]$ ctags -f cmake.tags --options=masatake_cmake_2.ctags  --totals=yes -L cmake.files
349 files, 27601 lines (999 kB) scanned in 9.6 seconds (103 kB/s)
1305 tags added to tag file
1305 tags sorted in 0.00 seconds

Is it worth it? (I'm not sure how popular the end: field is)

@masatake
Copy link
Author

Thank you for the optimization.
end: field is newly introduced in Universal-ctags. It is not in Exuberant-ctags. I think it is important information for client tools written in the future but, as far as I know, there is no client tool utilizing the fields.

I think you cannot accept the 10% impact for unused information.
To realize both gathering rich information and parsing speed, I think we have to improve the main part of ctags. The idea I get is introducing new flags realzing condtional pattern like:

...
--_mtable-regex-CMake=main/end(function|macro)[ \t]*\(//{icase}{tleave}{if-field-is-enabled=end}
...

The pattern is inserted to the "main" table only when "end" field is enabled when starting ctags.
The user who doesn't need the end field information runs ctags with --fields=-e option.(This is current default behavior.) So ctags parsers cmake files as fast as you originally wrote it.
The user who needs the information runs ctags with --fields=+e option instead.

I think I cannot implement such feature quickly. So I would like to withdraw this PR.
For studying, can I ask you to put improved version of .ctags file for recording end: field here?

@masatake masatake closed this Aug 11, 2018
@hadrielk
Copy link
Owner

I started modifying the cmake.ctags file to reference the scope info for variables when they're in functions, and doing that you get the end: info for free basically. Is the scoping info worth impacting performance? (do clients use the scope info?)

If so, I can update the PR with that change.

@masatake
Copy link
Author

The worth is really up to the client tools. I don't know client tools well. However, I believe such informations are useful.

One of the well known complaints about ctags is name confliction; an editor shows multiple tags for a name specified by a user. The user wants to jump to the definition for the name directly, but the client show candidates instead.

scope info and end info helps the developers of client tools implement the direct jump or reducing the candidates.

If so, I can update the PR with that change.

Though I opened this #1, I would like to merge universal-ctags#1818 first.
Can I ask you to open new one for the end/scope fields after I merge universal-ctags#1818 ?

@hadrielk
Copy link
Owner

Sure no problem.

@hadrielk
Copy link
Owner

To realize both gathering rich information and parsing speed, I think we have to improve the main part of ctags. The idea I get is introducing new flags realzing condtional pattern like:

--_mtable-regex-CMake=main/end(function|macro)[ \t]*\(//{icase}{tleave}{if-field-is-enabled=end}

The pattern is inserted to the "main" table only when "end" field is enabled when starting ctags.
The user who doesn't need the end field information runs ctags with --fields=-e option.(This is current default behavior.) So ctags parsers cmake files as fast as you originally wrote it.
The user who needs the information runs ctags with --fields=+e option instead.

I was actually trying something similar a while back: I tried controlling whether a --_mtable-regex-...= would be on/off by using the {_extra=...} flag. But obviously I found out it didn't actually disable the regex; it just disabled adding the tag for that regex line. In hindsight I decided it was actually good it didn't disable it, because that could significantly change the logic flow of the optlib file.

But yes some way to disable specific regex lines might be useful. (as a side note, I saw the function API for the matching functions in lregex.c all take a bool *disabled argument but nothing ever seems to set it to anything but NULL?)

As a side note: the reason I wanted it was I was thinking it would be cool to let users extend the tables created by cmake.ctags, so they could add regex to match their specific variables and tag them with what the variable was. But in testing a way to do that it became clear that wasn't going to work, because the user-supplied optlib .ctags file is loaded before the built-in cmake ones (which I guess they need to be, to handle the extra/kind/field info being known before the built-in stuff happens?). So then the user's .ctags file can't reference a regex table name from cmake because it doesn't exist yet, and if they create the table in their .ctags file then when the built-in cmake one is loaded it conflicts with the table name and errors out.

But really for something like that they can just copy/paste the cmake.ctags from github, modify the langdef name to be their own, and modify the logic as they want. 😄

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.

2 participants