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

Fully qualified c++ scopes would be a great feature #1547

Closed
GutHubert opened this Issue Sep 7, 2017 · 14 comments

Comments

Projects
None yet
3 participants
@GutHubert

GutHubert commented Sep 7, 2017

masatake I welcome latest changes in Ctags and am fond of your quick response! Thanks a lot!

Ctags Aug. 22 2017

It is hard to trace xref outputs having a qualified scope only besides it is prone to errors (implementation of example the same function in different namespaces same class name).

Fully qualified: Namespace::class:function........ (this is not implemented)
Qualified: class::function........ (this is implemented in various versions)

The trouble is seen for sorted as well as unsorted tag output. Even in unsorted output I see a function tag before the definition of the class tag. I run a categorized analysis (1. Namespaces, 2.Classes, 3.structures....) and run with a bigger project in trouble.

BTW: Regards to Alaska from nearby Equator.

@masatake

This comment has been minimized.

Member

masatake commented Sep 7, 2017

Could you show small example input C++ file?
I don't know C++ well. Real, small and well simplified example drives me.

Don't forget to use triple back quote chars to quote source code (and tags output) like

int main(void)
{
    return main();
}
@GutHubert

This comment has been minimized.

GutHubert commented Sep 7, 2017

@masatake

This comment has been minimized.

Member

masatake commented Sep 7, 2017

I don't understand what you wrote. The code in your post is difficult to read. Source code quotation with line-folded way killed me.

// Scintilla source code edit control
/** @file Accessor.h
 ** Interfaces between Scintilla and lexers.
 **/
// Copyright 1998-2010 by Neil Hodgson <neilh@scintilla.org>
// The License.txt file describes the conditions under which this software may be distributed.

#ifndef ACCESSOR_H
#define ACCESSOR_H

#ifdef SCI_NAMESPACE
namespace Scintilla {
#endif

enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };

class Accessor;
class WordList;
class PropSetSimple;

typedef bool (*PFNIsCommentLeader)(Accessor &styler, Sci_Position pos, Sci_Position len);

class Accessor : public LexAccessor {
public:
	PropSetSimple *pprops;
	Accessor(IDocument *pAccess_, PropSetSimple *pprops_);
	int GetPropertyInt(const char *, int defaultValue=0) const;
	int IndentAmount(Sci_Position line, int *flags, PFNIsCommentLeader pfnIsCommentLeader = 0);
};

#ifdef SCI_NAMESPACE
}
#endif

#endif
u-ctags --kinds-C++=+p --language-force=C++ --extras=+q -o - Accessor.h
u-ctags --kinds-C++=+p --language-force=C++ --extras=+q -o - Accessor.h
ACCESSOR_H	Accessor.h	/^#define ACCESSOR_H$/;"	d
Accessor	Accessor.h	/^	Accessor(IDocument *pAccess_, PropSetSimple *pprops_);$/;"	p	class:Scintilla::Accessor
Accessor	Accessor.h	/^class Accessor : public LexAccessor {$/;"	c	namespace:Scintilla
GetPropertyInt	Accessor.h	/^	int GetPropertyInt(const char *, int defaultValue=0) const;$/;"	p	class:Scintilla::Accessor	typeref:typename:int
IndentAmount	Accessor.h	/^	int IndentAmount(Sci_Position line, int *flags, PFNIsCommentLeader pfnIsCommentLeader = 0);$/;"	p	class:Scintilla::Accessor	typeref:typename:int
PFNIsCommentLeader	Accessor.h	/^typedef bool (*PFNIsCommentLeader)(Accessor &styler, Sci_Position pos, Sci_Position len);$/;"	t	namespace:Scintilla	typeref:typename:bool (*)(Accessor & styler,Sci_Position pos,Sci_Position len)
Scintilla	Accessor.h	/^namespace Scintilla {$/;"	n
Scintilla::Accessor	Accessor.h	/^class Accessor : public LexAccessor {$/;"	c	namespace:Scintilla
Scintilla::Accessor::Accessor	Accessor.h	/^	Accessor(IDocument *pAccess_, PropSetSimple *pprops_);$/;"	p	class:Scintilla::Accessor
Scintilla::Accessor::GetPropertyInt	Accessor.h	/^	int GetPropertyInt(const char *, int defaultValue=0) const;$/;"	p	class:Scintilla::Accessor	typeref:typename:int
Scintilla::Accessor::IndentAmount	Accessor.h	/^	int IndentAmount(Sci_Position line, int *flags, PFNIsCommentLeader pfnIsCommentLeader = 0);$/;"	p	class:Scintilla::Accessor	typeref:typename:int
Scintilla::Accessor::pprops	Accessor.h	/^	PropSetSimple *pprops;$/;"	m	class:Scintilla::Accessor	typeref:typename:PropSetSimple *
Scintilla::PFNIsCommentLeader	Accessor.h	/^typedef bool (*PFNIsCommentLeader)(Accessor &styler, Sci_Position pos, Sci_Position len);$/;"	t	namespace:Scintilla	typeref:typename:bool (*)(Accessor & styler,Sci_Position pos,Sci_Position len)
Scintilla::__anon194cc2ce0103	Accessor.h	/^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/;"	g	namespace:Scintilla
Scintilla::wsInconsistent	Accessor.h	/^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/;"	e	enum:Scintilla::__anon194cc2ce0103
Scintilla::wsSpace	Accessor.h	/^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/;"	e	enum:Scintilla::__anon194cc2ce0103
Scintilla::wsSpaceTab	Accessor.h	/^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/;"	e	enum:Scintilla::__anon194cc2ce0103
Scintilla::wsTab	Accessor.h	/^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/;"	e	enum:Scintilla::__anon194cc2ce0103
__anon194cc2ce0103	Accessor.h	/^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/;"	g	namespace:Scintilla
pprops	Accessor.h	/^	PropSetSimple *pprops;$/;"	m	class:Scintilla::Accessor	typeref:typename:PropSetSimple *
wsInconsistent	Accessor.h	/^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/;"	e	enum:Scintilla::__anon194cc2ce0103
wsSpace	Accessor.h	/^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/;"	e	enum:Scintilla::__anon194cc2ce0103
wsSpaceTab	Accessor.h	/^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/;"	e	enum:Scintilla::__anon194cc2ce0103

Scintilla::Accessor::Accessor is recorded well.
If you are talking about scope field, see #1510.

@GutHubert

This comment has been minimized.

GutHubert commented Sep 7, 2017

@masatake

This comment has been minimized.

Member

masatake commented Sep 7, 2017

... so what I should do?
I don't understand the direction of this issue. Are you talking about a bug or feature enhancement?
If, yes, an example is needed: input, cmdline and output.
Obviously a better editor is needed.

@GutHubert

This comment has been minimized.

GutHubert commented Sep 8, 2017

@b4n

This comment has been minimized.

Member

b4n commented Sep 8, 2017

I'm not sure I follow either, because given @masatake example, the scope is fully qualified just as you asked for (GetPropertyInt […] class:Scintilla::Accessor). Also, note that if the Scintilla namespace is guarded inside preprocessor conditionals, it also requires the parser to properly understand those conditionals as you would like it to, which is not necessarily easy: in this example, do the user want the namespace or not? it depends on the value of SCI_NAMESPACE, which is unknown.

@masatake

This comment has been minimized.

Member

masatake commented Sep 8, 2017

@b4n, thank you. From the your comment, I can guess the original request a bit.

Input:

#ifdef SCI_NAMESPACE
namespace Scintilla {
#endif

class Accessor {
public:
	Accessor();
};

#ifdef SCI_NAMESPACE
}
#endif

Cmdline:

u-ctags  --language-force=C++ -o - --kinds-C++=+p  --extras=+q  /tmp/foo.h

The output:

Accessor	/tmp/foo.h	/^	Accessor();$/;"	p	class:Scintilla::Accessor
Accessor	/tmp/foo.h	/^class Accessor {$/;"	c	namespace:Scintilla
Scintilla	/tmp/foo.h	/^namespace Scintilla {$/;"	n
Scintilla::Accessor	/tmp/foo.h	/^class Accessor {$/;"	c	namespace:Scintilla
Scintilla::Accessor::Accessor	/tmp/foo.h	/^	Accessor();$/;"	p	class:Scintilla::Accessor

YOUR expected output:

Accessor	/tmp/foo.h	/^	Accessor();$/;"	p	class:Accessor
Accessor	/tmp/foo.h	/^class Accessor {$/;"	c
Accessor::Accessor	/tmp/foo.h	/^	Accessor();$/;"	p	

Introducing -U optoin for undefining a cpp macro may help to get the expected output.

u-ctags  -U SCI_NAMESPACE --language-force=C++ -o - --kinds-C++=+p  --extras=+q  /tmp/foo.h

This is nothing to do with scope. This is just about cpreprocessing as @b4n wrote.
However, I'm not sure I understand the original requirement.

@masatake

This comment has been minimized.

Member

masatake commented Sep 8, 2017

#1536 is really beautiful issue. Quite understandable.

@GutHubert

This comment has been minimized.

GutHubert commented Sep 8, 2017

@b4n

This comment has been minimized.

Member

b4n commented Sep 9, 2017

Ctags traces namespaces in preproc directives just fine as output shows. SCI_NAMESPACE is in a header file and thus known besides commented as known.

CTags doesn't tracks inclusions, so it can only guess preprocessing branches. I think the new parser might (?) be using the defines it saw in the file and the ones from the options (-D), but I'm not even sure it does. If it doesn't it could be a nice addition, but it can also be a little too much work to add for not necessarily that much value as includes aren't processed.

Providing the whole source puts Masatake to the afterlife as he explains. We need him a bit longer, he's great! So what you see is narrowed down to the issue.

I think the problem was that your message is not formatted in a way that shows anything meaningful on GitHub (or in my email for that matter), maybe you could paste it again using GitHub's markup for code. I think that would keep @masatake's heart in check ;)

What's left is that the scope definition is used for symbols and seems to be correct there.

If the example @masatake and I see doesn't show the issue, it's no real help for understanding what you're searching for :)

Again this is ok for a code editor but wrong as scope definition.The reported scope in tags is a crumbled string of no value as of now.It causes havoc.

What is the problem with scopes like class:Scintilla::Accessor? how would you like it instead?

I myself see only one problem with it, which is that I would love to have a way to know which tag is the actual parent (e.g. more precisely than a name that could appear twice), but that's not always 100% possible for all languages (e.g. in C++ implementations it references scopes not declared in that file), and we'd need a new field/technique for that anyway (but that'd probably be a great feature one day: e.g. having a unique identifier for each tag, and using that identifier for knowing which tag is the parent).
In the current situation, it could be improved also by being able to query the separator used between the different identifiers (e.g. :: for the example above) so it could be possible to programatically figure all the info out (do we have a pseudo-tag for that already?), and perhaps having the kind for each element (which is a problem for compatibility though).

Since fully qualified symbols seem to be correct move the scope part of it into scope.

Hum, if the fully qualified tag names are OK, what's the problem, doesn't the scope match? AFAIK the C++ parser uses the exact same info for the scope and the qualifying part in FQ tags, so I don't understand where you're going at. Could you provide an example of what you have, and what you'd like?

@GutHubert

This comment has been minimized.

GutHubert commented Sep 11, 2017

Masatake, I have narrowed it down. This causes errors.
Accessor files attached

For some function and all parameter, local and some member tags.
Other tags supply the namespace in the scope.

(my code) Works ok here but is russian roulett.
Do a ns::class lookup to get the namespace from an internal list.
The class name is the first term of the scope.

  • Accessor tags as in the tags file normal sequence
  • Not Fully qualified tags: (namespace Scintilla is missing)
    Accessor.h header 18 |D:\Accessor.cxx| ???? /^#include "Accessor.h"/ ![-]!
    Accessor function 24 |D:\Accessor.cxx| class ??Accessor?? /^Accessor::Accessor(IDocument *pAccess_, PropSetSimple *pprops_) : LexAccessor(pAccess_), pprops(/ ![-]!
    pAccess_ parameter 24 |D:\Accessor.cxx| function ??Accessor::Accessor?? /^Accessor::Accessor(IDocument *pAccess_, PropSetSimple *pprops_) : LexAccessor(pAccess_), pprops(/ ![IDocument *]!
    pprops_ parameter 24 |D:\Accessor.cxx| function ??Accessor::Accessor?? /^Accessor::Accessor(IDocument *pAccess_, PropSetSimple *pprops_) : LexAccessor(pAccess_), pprops(/ ![PropSetSimple *]!
    GetPropertyInt function 27 |D:\Accessor.cxx| class ??Accessor?? /^int Accessor::GetPropertyInt(const char *key, int defaultValue) const {$/ ![int]!
    key parameter 27 |D:\Accessor.cxx| function ??Accessor::GetPropertyInt?? /^int Accessor::GetPropertyInt(const char *key, int defaultValue) const {$/ ![const char *]!
    defaultValue parameter 27 |D:\Accessor.cxx| function ??Accessor::GetPropertyInt?? /^int Accessor::GetPropertyInt(const char *key, int defaultValue) const {$/ ![int]!
    IndentAmount function 31 |D:\Accessor.cxx| class ??Accessor?? /^int Accessor::IndentAmount(Sci_Position line, int *flags, PFNIsCommentLeader pfnIsCommentLeader)/ ![int]!
    line parameter 31 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^int Accessor::IndentAmount(Sci_Position line, int *flags, PFNIsCommentLeader pfnIsCommentLeader)/ ![Sci_Position]!
    flags parameter 31 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^int Accessor::IndentAmount(Sci_Position line, int *flags, PFNIsCommentLeader pfnIsCommentLeader)/ ![int *]!
    pfnIsCommentLeader parameter 31 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^int Accessor::IndentAmount(Sci_Position line, int *flags, PFNIsCommentLeader pfnIsCommentLeader)/ ![PFNIsCommentLeader]!
    end local 32 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^ const Sci_Position end = Length();$/ ![const Sci_Position]!
    spaceFlags local 33 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^ int spaceFlags = 0;$/ ![int]!
    pos local 40 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^ Sci_Position pos = LineStart(line);$/ ![Sci_Position]!
    ch local 41 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^ char ch = (*this)[pos];$/ ![char]!
    indent local 42 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^ int indent = 0;$/ ![int]!
    inPrevPrefix local 43 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^ bool inPrevPrefix = line > 0;$/ ![bool]!
    posPrev local 44 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^ Sci_Position posPrev = inPrevPrefix ? LineStart(line-1) : 0;$/ ![Sci_Position]!
    chPrev local 47 |D:\Accessor.cxx| function ??Accessor::IndentAmount?? /^ const char chPrev = (*this)[posPrev++];$/ ![const char]!
    Ctags Errors.zip

ACCESSOR_H macro 9 |D:\Accessor.h| ???? /^#define ACCESSOR_H$/ ![-]!

  • Fully qualified tags: (from here onwards it is ok)
    Scintilla namespace 12 |D:\Accessor.h| ???? /^namespace Scintilla {$/ ![-]!
    __anon001ee93c0103 enum 15 |D:\Accessor.h| namespace ??Scintilla?? /^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/ ![-]!
    wsSpace enumerator 15 |D:\Accessor.h| enum ??Scintilla::__anon001ee93c0103?? /^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/ ![-]!
    wsTab enumerator 15 |D:\Accessor.h| enum ??Scintilla::__anon001ee93c0103?? /^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/ ![-]!
    wsSpaceTab enumerator 15 |D:\Accessor.h| enum ??Scintilla::__anon001ee93c0103?? /^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/ ![-]!
    wsInconsistent enumerator 15 |D:\Accessor.h| enum ??Scintilla::anon001ee93c0103?? /^enum { wsSpace=1, wsTab=2, wsSpaceTab=4, wsInconsistent=8 };$/ ![-]!
    PFNIsCommentLeader typedef 21 |D:\Accessor.h| namespace ??Scintilla?? /^typedef bool (PFNIsCommentLeader)(Accessor &styler, Sci_Position pos, Sci_Position len);$/ ![bool ()(Accessor & styler,Sci_Position pos,Sci_Position len)]!
    Accessor class 23 |D:\Accessor.h| namespace ??Scintilla?? /^class Accessor : public LexAccessor {$/ ![-]!
    pprops member 25 |D:\Accessor.h| class ??Scintilla::Accessor?? /^ PropSetSimple *pprops;$/ ![PropSetSimple *]!
    Accessor prototype 26 |D:\Accessor.h| class ??Scintilla::Accessor?? /^ Accessor(IDocument *pAccess
    , PropSetSimple *pprops
    );$/ ![-]!
    GetPropertyInt prototype 27 |D:\Accessor.h| class ??Scintilla::Accessor?? /^ int GetPropertyInt(const char *, int defaultValue=0) const;$/ ![int]!
    IndentAmount prototype 28 |D:\Accessor.h| class ??Scintilla::Accessor?? /^ int IndentAmount(Sci_Position line, int *flags, PFNIsCommentLeader pfnIsCommentLeader = 0);$/ ![int]!

xfmt = "-x --_xformat="%N %z %n |%F| %p ??%s?? %P ![%t]! " ";
xtra = "--extras=+r ";
langKindCmd = "−−C++−kinds=+d+e+f+g+h+l+m+p+s+t+u+v+x+z+L+c+n+A+N+U "
cmdLine = "−−sort=no " + xfmt + langKindCmd + xtra + excludeFiles + files;

--extras=+q creates double entries, it's not the way to go.(Scintilla: 21'250 vs. 15'700)
Accessor function 24 ...
Accessor::Accessor function 24 ...

I am running automated tagging over multiple projects for:
ASM, C++, C#

@masatake

This comment has been minimized.

Member

masatake commented Sep 11, 2017

More requests:

  • narrow down the input. Accessor.h is too large.
  • use triple backquotes and quote the input. Learn GitHub's markup.
  • don't use xfmt instead tags output if possible.
  • don't make me read the long list. Show only a few example.
  • see #1536. It is really beautiful issue report. Quite understandable.
  • ,...

@masatake masatake closed this Sep 11, 2017

@masatake

This comment has been minimized.

Member

masatake commented Sep 11, 2017

I wll never read unformatted long list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment