This is far from perfect and contains a lot of guessing. It showed good results based on our tests cases, fixing several issues and not introducing any more issues (admittedly, after working around a subtle one regarding D static ifs).
Closes #845. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/889
-- Commit Summary --
* C, C++, C#, D: Improve return type and var type recognition
-- File Changes --
M tagmanager/ctags/c.c (70) M tests/ctags/Makefile.am (1) M tests/ctags/bit_field.c.tags (8) M tests/ctags/bug1799340.cpp.tags (6) M tests/ctags/bug1907083.cpp.tags (8) M tests/ctags/bug1924919.cpp.tags (4) M tests/ctags/c-digraphs.c.tags (4) M tests/ctags/c-trigraphs.c.tags (4) M tests/ctags/indexer.cs.tags (2) M tests/ctags/interface_indexers.cs.tags (2) M tests/ctags/keyword_const.cs.tags (4) M tests/ctags/keyword_virtual.cs.tags (2) M tests/ctags/keyword_volatile.cs.tags (2) M tests/ctags/simple.d.tags (2) A tests/ctags/var-and-return-type.cpp (40) A tests/ctags/var-and-return-type.cpp.tags (20)
-- Patch Links --
https://github.com/geany/geany/pull/889.patch https://github.com/geany/geany/pull/889.diff
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889
@elextr please look, check, tests and see it's a little less wrong than before :)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889#issuecomment-175073006
@techee thanks! Beware, it contains its fair share of hacks (it's c.c, so no surprise).
It's so much of a mess I almost seriously considered trying to rewriting if from scratch, properly split and everything. I'm pretty sure in the end it would save me time, instead of fixing this again and again, with low confidence on the impacts each iteration can have…
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889#issuecomment-175081628
+};
+const struct type1 func2(); +const struct type1 var2 = { 0, 0 };
+typedef type1 type1_t;
+const type1_t func3(); +const type1_t var3 = { 0, 0 };
+struct type1 func4(); +struct type1 var4 = { 0, 0 };
+type1_t func5(); +type1_t var5 = { 0, 0 };
what about testing `type1_t var{};` or `type1_t var{ 0, 0 };`
what about testing `auto func()->type1_t` C++11 syntax
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50914947
+};
+const struct type1 func2(); +const struct type1 var2 = { 0, 0 };
+typedef type1 type1_t;
+const type1_t func3(); +const type1_t var3 = { 0, 0 };
+struct type1 func4(); +struct type1 var4 = { 0, 0 };
+type1_t func5(); +type1_t var5 = { 0, 0 };
what about testing `type1_t var{};` or `type1_t var{ 0, 0 };`
Nothing to do with reporting the var type, it's "just" another mean of initializing. So different issue, possibly different test case.
what about testing `auto func()->type1_t` C++11 syntax
This is more related, but still quite outside the scope here, as AFAIK `->` syntax is not supported at all yet. (OT, WTF is about this syntax? why add a new one?)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50917223
+};
+const struct type1 func2(); +const struct type1 var2 = { 0, 0 };
+typedef type1 type1_t;
+const type1_t func3(); +const type1_t var3 = { 0, 0 };
+struct type1 func4(); +struct type1 var4 = { 0, 0 };
+type1_t func5(); +type1_t var5 = { 0, 0 };
Nothing to do with reporting the var type, it's "just" another mean of initializing. So different issue, possibly different test case.
Fair enough
AFAIK -> syntax is not supported at all yet.
For some reason I thought it was, maybe another language.
(OT, WTF is about this syntax? why add a new one?)
So it is after parameter declaration and so can use decltype(parameter) where the parameter type is from a template parameter and so not known until template parameter deduction.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50917934
- if (! st->gotArgs)
return vStringValue(st->firstToken->name); /* ignore non-functions */
switch (st->declaration) {
case DECL_BASE:case DECL_FUNCTION:case DECL_FUNCTION_TEMPLATE:break;default:return vStringValue(st->firstToken->name);}
if (vt == NULL)
vt seems to be always NULL (not really related to your changes).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50965061
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
Just for my information, what do the two lines check? Or in other words, what would happen if the check was missing and we always ended at nameToken? (I don't completely understand the comment above.)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50965800
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
OK, is it to strip away templates? Maybe we could keep them here and remove them in the strip_type() function in TM. On the other hand one usually doesn't want to see the multiline templates from std:: at all :-)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50966540
@b4n I had a brief look at the patch and it good. Well, can't say I really get the subtle things but the overall logic looks good and the test results look good too.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889#issuecomment-175545466
@b4n I'm wondering if it wouldn't be easier not to process the tokens for var_type but just the raw input similarly to getArglistFromFilePos() and stripping away comments, newlines and double-spaces. This seems to be a similar case. But I may miss something.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889#issuecomment-175550791
OK, thinking about it more by just recording the string we wouldn't strip away things like public/private/... which aren't really a type. So yeah, I guess the current approach is right.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889#issuecomment-175553783
- if (! st->gotArgs)
return vStringValue(st->firstToken->name); /* ignore non-functions */
switch (st->declaration) {
case DECL_BASE:case DECL_FUNCTION:case DECL_FUNCTION_TEMPLATE:break;default:return vStringValue(st->firstToken->name);}
if (vt == NULL)
no, it's `static` (which is meh, but well)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50978248
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
No, the point is to not include e.g. the class name in `std::string myclass::do_something();`. The idea is fairly simple: when I see a name not preceded by a double colon (e.g. not simply qualified), and I already found something looking like a type (either a custom name, or a data type keyword (`void`, `int`, etc.)), I assume it's not part of the type anymore.
Maybe I miss some legitimate cases I don't know about, and yeah, it might lead to also ignoring template types (but it already did before, so it might be unrelated and that might just not show in the statement's tokens).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50979636
@techee well, maybe… but honestly I'm not sure how I'd do that, nor if it didn't indeed produce a lot of uninteresting stuff that'd be just as hard to cleanup. And it doesn't solve the problem of "how do I know where the return type starts and ends" :)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889#issuecomment-175606563
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
@b4n
will `typename foo<t1, t2>::a type qualifier::func()` work?
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50980376
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
Probably not, gives type `typename foo::a` (to `qualifier::func()` function).
Maybe `typename` could be handled specially if needed? Or maybe another solution, like finding the name token first, and then going back to skip the possible qualifier, might give better results? it's what I first tried and it was a mess, but maybe it could be made to work.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50980765
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
@b4n, did you get the version with my typo fixed? (damn github for not notifying edits)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50981094
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
ah, no, I was too quick :) I picked it on the webpage so it ain't github's fault this time, but no I used the version with the space.
Your corrected example gives type `typename foo::a_type` (which looks more or less correct to my innocent eyes, except for the stripped `<>`, which I believe is an issue of what gets put in the statement's token list).
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50981463
- if (! st->gotArgs)
return vStringValue(st->firstToken->name); /* ignore non-functions */
switch (st->declaration) {
case DECL_BASE:case DECL_FUNCTION:case DECL_FUNCTION_TEMPLATE:break;default:return vStringValue(st->firstToken->name);}
if (vt == NULL)
Uh, right, I missed that.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50982900
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
@b4n Thanks for the clarifications - the code looks good to me and seems to work fine in most cases.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50983080
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
@b4n, leaving out the template parameters is sort of bad, they really are part of the type. Makes it quite long of course, but showing `std::map` for `std::map<std::string, std::pair<std::string, int>>` is missing a lot of information.
Also another test that has parens and semi-arbitary expressions in the type :)
``` decltype(std::declval<type_with_member_function_foo>().foo()) n2 = n1; ```
I am assuming trying these sorts of things now is better than waiting until you finish and then point out something else missing :)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50983640
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
@elextr I'm afraid that proper template parsing would double the c.c size and would take a really lot of time to develop. The only possibility to have templates in the type is to really just record the input string together with some reasonable begin/end type detection. But even this will probably require quite some work and the question is whether it's worth it.
The implementation in this patch is good enough IMO - at least for the scope completion where we'd have to remove the templates anyway.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r50985317
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
To summarise our IRC discussion while github was down.
@b4n is concerned about the template params in autocompletion, I'm concerned about them in the tooltips. Maybe those need to be separate things, with the tooltip being just a copy of the relevant part of the source as suggested above.
BTW I am not suggesting parsing the parameters, I don't want compiler like situations where `std::string` is expanded to `std::basic_string<char, traits blah blah>`.
The decltype example fails as expected.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r51076674
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
@b4n is concerned about the template params in autocompletion, I'm concerned about them in the tooltips.
I'm not really concerned about one side or the other, just about reporting something relevant. Actually, I'd rather be concerned by reporting relevant tooltips, and then when it works figure out how the heck we can use that for completion.
Maybe those need to be separate things, with the tooltip being just a copy of the relevant part of the source as suggested above.
Indeed, maybe. Though, I tried to "just [make] a copy of the source", and it seems a lot easier said than done, because for some reason the token's position don't seem to be really correct and I get more or less garbage that way. I dropped it after pulling my last hair. I might give it another shot when they have grew back :)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r52606297
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
@b4n I agree - it's fine the way it is. What about merging this plus the bloody scope completion patch set so the whole thing is over? I'd like to push some related patches like autocompletion for namespaces plus some cleanup patches but I'd like to have it on top of master with scope completion in.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r52607192
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
I'm actually on it right now, should pop up in master anytime now :)
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r52607370
for (i = 0; i < st->tokenIndex; i++) {
const tokenInfo *const t = st->token[i];/* stop if we find the token used to generate the tag name, or* a name token in the middle yet not preceded by a scope separator */if ((t == nameToken ||(t->type == nameToken->type &&t->keyword == nameToken->keyword &&t->lineNumber == nameToken->lineNumber &&strcmp(vStringValue(t->name), vStringValue(nameToken->name)) == 0)) ||(t->type == TOKEN_NAME && seenType &&(i > 0 && st->token[i - 1]->type != TOKEN_DOUBLE_COLON)))
Yay, great!!!
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889/files#r52607506
Merged #889.
--- Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/889#event-547225071
github-comments@lists.geany.org