New code completion remarks/issues

[ollydbg]

Wait, there is a big syntax error in the if statement.

Code

 if (   !SkipToOneOfChars(_T(">\r\n")), false      )

look at the pair of parentheses!!!?

it is

Code

if ( XXXX, false)

[ollydbg]

Patch to fix the "template reading bug" , this patch contains some patch in CC comments improvement



[attachment deleted by admin]

[MortenMacFly]

Patch to fix the "template reading bug" ,

Wow, that was fast (I didn't even manage to run C::B today). IF this works that would be nice!

BTW: Please don't include other patches next time, if possible. This makes merging for me a little easier. ;-)

[ollydbg]

hi, morten, I have an idea, I want to hear your comments.

Currently, the Tokenizer is only return a wxString as a Token.
But my idea is, Tokenizer should return a Token structure.

For example

Code

void f(XXX);

In the current trunk code, the Tokenizer will return four wxString.

Code

void
f
(XXX)
;

But my suggestion is to create a Token structure like

Code

struct Token{
wxString name;
Tokentype type;
}

So, the Tokenizer::GetToken will return four strictures, because When Tokenizer do the "DoGetToken" function, it is already know the Token's type, so, we don't need the check the wxString in ParserThread.(mostly, we need to check the first character of the wxstring to determine this is a (XXXX)).

Code

void its type is normalText
f     its type is normalText
(XXX) its type is functionArgument
;       its type is endofStatement

This will make parserThread more happier and easy to determine the syntax.


Edit

following my idea, the DoGetToken can be changed to like below:

Code

    else if (CurrentChar() == '<' && bTemplate)
    {
        MoveToNextChar();
        if (!SkipToOneOfChars(_T(">")), true)
            return wxEmptyString;

        MoveToNextChar();
        wxString tmp = m_Buffer.Mid(start+1,m_TokenIndex-start-2);
        tmp.Trim();
        str = _T("<");
        str += tmp;
        str += _T(">"); // m_Buffer.Mid(start, m_TokenIndex - start);
        type = TemplateArgument;
    }
    else if (c == '(')
    {
        m_IsOperator = false;

        // skip blocks () []
        if (!SkipBlock(CurrentChar()))
            return wxEmptyString;

        str = FixArgument(m_Buffer.Mid(start, m_TokenIndex - start));
        CompactSpaces(str);
        type = functionArgument;
    }
    else
    {
        if      (c == '{')
            ++m_NestLevel;
            type = OpenBrace
        else if (c == '}')
            --m_NestLevel;
            type = CloseBrace

        str = c;
        MoveToNextChar();
    }

Take care of the "type" variable, it is a enum variable, can have the type like:

Code

enum Tokentype{

normalText;
OpenBrace;
CloseBrace;
EndOfStatement;
FunctionArgument;
TemplateArgument;
....

}

So, in the parserThread void ParserThread::DoParse, we can use a switch statement

Code

switch(type){

case normaltext: XXXX
case:functionArgument: XXXX


}

This will runs faster then the many "if - else if ....."

[blueshake]

Patch to fix the "template reading bug" , this patch contains some patch in CC comments improvement

forget to do this in patch.(will make the parser in a long work time.) :lol:

Code

#define PARSERTHREAD_DEBUG_OUTPUT 0

[blueshake]

hi,mortern:
can you comment these codes,because they make no sense.

Code

Index: src/plugins/codecompletion/parser/parserthread.cpp
===================================================================
--- src/plugins/codecompletion/parser/parserthread.cpp	(revision 5986)
+++ src/plugins/codecompletion/parser/parserthread.cpp	(working copy)
@@ -661,17 +661,17 @@
                     && (!m_pLastParent || m_pLastParent->m_Name != token) ) // if func has same name as current scope (class)
                 {
                     // test Macro
-                    if (token.Upper() == token)
+                    /*if (token.Upper() == token)
                     {
                         HandleMacro(token, peek);
                         m_Tokenizer.GetToken();
                         m_Str.Clear();
                     }
                     else
-                    {
+                    {*/
                         wxString arg = m_Tokenizer.GetToken(); // eat args ()
                         m_Str = token+arg;
-                    }
+                    //}
                 }
                 else if (peek.GetChar(0) == '(' && m_Options.handleFunctions)
                 {

[MortenMacFly]

can you comment these codes,because they make no sense.

Why exactly does that make no sense? Because if commented macros are not being handeled at all.

[blueshake]

check this

Code

if (token.Upper() == token)

it make a hypothesis here that all macro should be capital,but if the macros are not capital letters,this will not work.

for example:

LINUX(ARG)  ---------------work.


linUX(ARG)    ----------------not work


and in this situation,should in using macro but not defining a macro.so it is no reason to add macro here.

[MortenMacFly]

LINUX(ARG)  ---------------work.
linUX(ARG)    ----------------not work

This is true, however, macros are usually uppercase it's like a rule everybody should follow. In addition it worked nicely for me for quite some macros that were CC'ed very nicely afterwards. However, I agree that the detection algorithm should be more precise. So I'll comment it for a while and add a ToDo there...

Pending changes will be committed a little later... I am still testing some other CC related stuff (including your other patch...).

[Techy]

Hi,

I started experiencing quite serious performance issue after rev. 5945 for big projects. To be more precise - the code that 5945 introduced doesn't cause the problem - it just made the problem more visible. What happened after this commit was that the number of enums found in the project increased to about 18000 (this looks like a correct number - I was grepping through the whole project and there are about 60000 occurrences of enum there - some possibly comments so this seems to be more or less OK). Parsing is quite fast (about 10s), but then the whole GUI freezes for about 40 seconds (CPU 100%). This happens even when re-saving a file (parsing 0.1s), but again the whole 40s freeze of the GUI. I experienced this before as well, but the freeze took only 2 seconds so I thought it was the old Linux I was using.

OK, what happens - parsing is really fast and because it runs in a separate thread, id doesn't block the GUI. However, when parsing is finished, NativeParser::OnParserEnd is invoked and there parser->LinkInheritance(false) is called, which is the thing that takes the terrible amount of time. So my question is:

1. Could this function be called only when "Display inheritance info" is selected in the options page or "Show inherited members" is selected from the right-click menu? Is the info used by calling this function used by something else as well? (I'm running codeblocks with the problematic command commented-out now without any major issue so I guess this should be possible). It would be good to be able to at least disable it.

2. Could this thing be performed in the parser thread? This at least wouldn't freeze the GUI. (Again, I think this might be possible as well.)

3. Could this function be done in a smarter way? For instance building the complete tree only once at the beginning and adding the classes from the saved file to the hierarchy. I haven't studied the algorithm of this thing in detail yet but I think it has quadratic complexity (at least when thinking how to implement it, the most trivial way has quadratic complexity). This maybe could be improved as well. Finally, I've seen that TokensTree::RecalcData() goes through all classes, typedefs and enums - only typedefs for classes and structs are interesting for this function so the rest could be filtered out. Why are the enums there? (Again, I haven't studied this in detail so maybe the above is done already - just putting some ideas I have here).

Your opinion?

[MortenMacFly]

1. Could this function be called only when "Display inheritance info" is selected in the options page or "Show inherited members" is selected from the right-click menu?
2. Could this thing be performed in the parser thread?
3. Could this function be done in a smarter way?

Your opinion?

To make it simple: 1.) Yes, 2.) Yes, 3.) Yes.

However, I didn't realise such a bottleneck with enums. What project are you talking about? Is this a public one so I can try?

[Techy]

First please replace the word "enum" with "typedef" in my previous post - the typedefs and classes are those problematic things...

(But I'm still wondering why the enums are there in RecalcData() [and typedefs as well] - it's a bit array so [without looking at the parser itself] I guess that for "typedef struct" you set both tkClass and tkTypedef so you can test for tkClass only here...)

Well, the project I'm working on isn't free so you cannot try it. However, the good news is that the problematic part of the project is the boost library it uses (the source tree contains a partial copy of boost). Download the full sources from here:

http://www.boost.org/

and add all the source files recursively to a project. With full copy of boost I wasn't even patient enough to wait until the tree is generated and killed codeblocks... (let me know if you are able to reproduce it)

I've also analysed this behaviour a bit: the part marked as     

// first loop to convert ancestors string to token indices for each token

takes about 7 seconds - even though this not so much compared to the total time, I think that this can be done completely during the parsing stage and not being reparsed every time (anyway, this would have to be done in a thread not to block the GUI).

But the most problematic part is

// second loop to calculate full inheritance for each token

I think that the recursive algorithm you use is suboptimal - for every class you compute its ancestors again and again, but you could reuse the work you have already done for the subclasses. With the copy of boost my project uses, the maximal depth of the inheritance hierarchy was 74. After thinking about it a bit (and getting some graph algorithms from the back of my brain), this should be much more optimal algorithm:

1. Set X equal to the set of all classes and Y equal to empty set
2. Set Z equal to the set of all classes that do not inherit anything
3. Set the set of all ancestors and descendants of all members of Z equal to empty set

do while X is getting smaller:
    4. Set X = X - Z, Y = Y union Z, and Z = emptyset
    5. Set Z equal to the set of all classes that inherit from some class from Y (in the case of multiple inheritance all the ancestors have to be from Y - otherwise don't put the class to Z)
    6. For every member M of Z set the set of all ancestors equal to the set of ancestors of the classes it inherits from (those from Y) plus the classes themselves. At the same time, for each of the ancestors of M add M as its descendant.

(In correctly written C++ program X should be empty set but before compiling the user can make an error so there is a loop in the inheritance. This is the reason why there is the condition "do while X is getting smaller")

The advantage is clear - you go up from the classes that don't inherit anything and reuse what you did in the previous step (and in addition, you avoid recursion which can speed up things a bit as well)

So a good start would be to implement this algorithm and see how much it helps (right now I don't have time to do this myself, sorry). If it's not enough (even if the time reduces to 1s, the freeze is still quite unpleasant), then the function should be put into the parser thread.

[Techy]

Just one more note about data types when implementing it - X can be a list of pointers to tokens, Y should be a map indexed by token index with a pointer to the token as the contents (we'll be mainly searching in it so it should be a map) and Z will not exist at all (you'll take members of X one by one and either move them to Y or leave them in X).

[oBFusCATed]

Simple example that breaks the completion. Place the code below in the main.cpp of new console project:

Code

class AClass
{
    AClass(int a, int b)
    {
    }
    int method(int b) { return 0; }
};

int main()
{
    AClass a(5, 6);
    a.method(1);

    return 0;
}

Steps
0. Save the file
1. Place the currsor at the end of line "a.method(1);"
2. Press enter
3. Type "a."
4. Press ctrl+space
5. Nothing shows

Also the calltip for the constructor call doesn't work
Using latest version from trunk (6023).

[Jenna]

Simple example that breaks the completion. Place the code below in the main.cpp of new console project:

Code

class AClass
{
    AClass(int a, int b)
    {
    }
    int method(int b) { return 0; }
};

int main()
{
    AClass a(5, 6);
    a.method(1);

    return 0;
}

Steps
0. Save the file
1. Place the currsor at the end of line "a.method(1);"
2. Press enter
3. Type "a."
4. Press ctrl+space
5. Nothing shows

Also the calltip for the constructor call doesn't work
Using latest version from trunk (6023).

Works fine here (debian 64-bit).