Rev9537 introduce a bug which will two class labels in the symbol browser

[ollydbg]

@Alpha and all.

The steps to reproduce the bug:

1, set the symbol browser view option to show "current file's tokens"
2, open Codeblocks.cbp, let the parser finish its job
3, Open a header file, such as: plugins\codecompletion\classbrowserbuilderthread.h
4, edit the header file, and save it
5, see the updated symbol browser tree, you can see duplicated class labels.

Reason:
Rev9537 add inherit information for the class, saved in m_Args, but once a file get reparsed, it first try to delete the class definition token, since the class token shared between h/cpp, this does not actually remove the token, just adjust the m_File and m_Line info. Later when parsing the header, the class token should be reused. After the rev9537, the bold line is not true any more.

Code

    // none of the above; check for token under parent
    if (!newToken)
    {
        newToken = TokenExists(newname, baseArgs, m_LastParent, kind);
        if (newToken)
        {
            TRACE(_T("DoAddToken() : Found token (parent)."));
            // Special handling function implementation, see comments above
            if (isImpl && (kind & tkAnyFunction))
                newToken->m_Args = args;
        }
    }

This will create a new Token, not re-use the old one.



Another bug I see is that void TokenTree::RemoveFile(int fileIdx) called twice when a file is reparsed. One in GUI handler, one in thread pool's task (canparse = m_TokenTree->ReserveFileForParsing(filename, true) != 0;)

No time to dig further......

[ollydbg]

To solve this, I suggest: avoid the data duplication, which is:
Do not save ancestor information in the m_Args, since we can simply get those information from m_AncestorsString or m_DirectAncestors. Also, template information (template parameter) can be get from m_TemplateArgument.

Another hack is to disable the check of m_Args in the function TokenExists() for a class kind Token, since those are used to check/merge overload functions.

[Alpha]

In my commit:

Code

http://cb.biplab.in/websvn/comp.php?repname=codeblocks&compare[]=/@9536&compare[]=/@9537

I had sent this data to m_Args because, from tracing through the rest of the code, I found there were already special handlers checking for it (but were previously useless, as m_Args was always empty for tkClass tokens).

Do not save ancestor information in the m_Args, since we can simply get those information from m_AncestorsString or m_DirectAncestors. Also, template information (template parameter) can be get from m_TemplateArgument.

I agree that this is probably the 'correct' solution (though, it may be difficult to do, as I seem to remember m_DirectAncestors not holding the necessary information).

[ollydbg]

In my commit:

Code

http://cb.biplab.in/websvn/comp.php?repname=codeblocks&compare[]=/@9536&compare[]=/@9537

I had sent this data to m_Args because, from tracing through the rest of the code, I found there were already special handlers checking for it (but were previously useless, as m_Args was always empty for tkClass tokens).

I see. Here is the patch to fix the bug, we can still use m_Args in class kind tokens.

Code

1d8e6ea1388b71b37ef900f3303cf07f1b91fc26
 src/plugins/codecompletion/parser/parserthread.cpp |  3 ++-
 src/plugins/codecompletion/parser/tokentree.cpp    | 12 +++++++++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/plugins/codecompletion/parser/parserthread.cpp b/src/plugins/codecompletion/parser/parserthread.cpp
index ef73481..e9630b3 100644
--- a/src/plugins/codecompletion/parser/parserthread.cpp
+++ b/src/plugins/codecompletion/parser/parserthread.cpp
@@ -1374,7 +1374,8 @@ Token* ParserThread::DoAddToken(TokenKind       kind,
     if (   newToken
         && (newToken->m_TemplateArgument == m_TemplateArgument)
         && (   kind & tkAnyFunction
-            || newToken->m_Args == args ) )
+            || newToken->m_Args == args
+            || kind & tkAnyContainer ) )
     {
         ; // nothing to do
     }
diff --git a/src/plugins/codecompletion/parser/tokentree.cpp b/src/plugins/codecompletion/parser/tokentree.cpp
index 4c303c4..e57747d 100644
--- a/src/plugins/codecompletion/parser/tokentree.cpp
+++ b/src/plugins/codecompletion/parser/tokentree.cpp
@@ -186,9 +186,11 @@ int TokenTree::TokenExists(const wxString& name, const wxString& baseArgs, int p
         if (!curToken)
             continue;
 
+        // for a container token, there args member variables are used to store other information
+        // so, don't compare args for tkAnyContainer
         if (   (curToken->m_ParentIndex == parent)
             && (curToken->m_TokenKind   == kind)
-            && (curToken->m_BaseArgs    == baseArgs) )
+            && (curToken->m_BaseArgs == baseArgs || kind & tkAnyContainer) )
         {
             return result;
         }
@@ -249,7 +251,11 @@ int TokenTree::TokenExists(const wxString& name, const wxString& baseArgs, const
         if (!curToken)
             continue;
 
-        if (curToken->m_TokenKind == kind && curToken->m_BaseArgs == baseArgs)
+        // for a container token, there args member variables are used to store other information
+        // so, don't compare args for tkAnyContainer
+        if (  curToken->m_TokenKind == kind
+            && (   curToken->m_BaseArgs == baseArgs
+                || kind & tkAnyContainer ))
         {
             for ( TokenIdxSet::const_iterator pIt = parents.begin();
                   pIt != parents.end(); ++pIt )
@@ -626,7 +632,7 @@ bool TokenTree::CheckChildRemove(const Token* token, int fileIdx)
         else
             return false;  // one child is belong to another file
     }
-    return true;           // no children should be reserved, so we can safely remov the token
+    return true;           // no children should be reserved, so we can safely remove the token
 }
 
 void TokenTree::RecalcFreeList()

But I'm not sure the patch above cause other bugs.

Do not save ancestor information in the m_Args, since we can simply get those information from m_AncestorsString or m_DirectAncestors. Also, template information (template parameter) can be get from m_TemplateArgument.

I agree that this is probably the 'correct' solution (though, it may be difficult to do, as I seem to remember m_DirectAncestors not holding the necessary information).

What information is not hold in m_DirectAncestors? Can you be more specific?

[ollydbg]

...
I see. Here is the patch to fix the bug, we can still use m_Args in class kind tokens.
...
But I'm not sure the patch above cause other bugs.

I commit the patch to trunk now rev 9640, fell free to adjust. I don't want the next nightly build version have such bug.