Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

Several improvements to Code Completion plugin

<< < (28/29) > >>

Huki:
@SteelRat: The inactive code highlighting is provided by Scintilla and only basic features are supported: only tokens defined and used within the same file, and only #ifdef (and simple #if cases without any arithmetic?).
The only way to provide proper support for this feature is to somehow interface scintilla library with the codecompletion one, so scintilla can talk to our CC (not the other way around..). For this you can look at some old discussions in this topic where I've provided some patches (if you're ok with building your own CB). Not sure if they still work with latest revisions though.. but I'll see about making them available again.

BTW, Scintilla is open source. :)

ollydbg:
I'm thinking that the code:

--- Code: ---bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken)
{
    ...
    ...
    // Fix token index
    m_TokenIndex -= bufferLen;

    // Reset undo token
    m_SavedTokenIndex   = m_UndoTokenIndex = m_TokenIndex;
    m_SavedLineNumber   = m_UndoLineNumber = m_LineNumber;
    m_SavedNestingLevel = m_UndoNestLevel  = m_NestLevel;

    // Update the peek token
    if (m_PeekAvailable && updatePeekToken)
    {
        m_PeekAvailable = false;
        // we set the m_PeekAvailable after calling DoGetToken() function inside the PeekToken()
        // to prevent unnecessary recursive call of PeekToken() function.
        PeekToken();
    }

    return true;
}

--- End code ---

Should be changed to


--- Code: ---bool Tokenizer::ReplaceBufferText(const wxString& target)
{
    ...
    ...
    // Fix token index
    m_TokenIndex -= bufferLen;

    // Reset undo token
    m_SavedTokenIndex   = m_UndoTokenIndex = m_TokenIndex;
    m_SavedLineNumber   = m_UndoLineNumber = m_LineNumber;
    m_SavedNestingLevel = m_UndoNestLevel  = m_NestLevel;

    // Update the peek token
    if (m_PeekAvailable)
        m_PeekAvailable = false;


    return true;
}

--- End code ---

Reason:
1, After some debugging, I see that once the buffer has changed, peek should be invalid, the following "PeekToken() or GetToken()" function call will update the peek token, but not inside the ReplaceBufferText() function.

2, I even think the last parameter of "bool ReplaceFunctionLikeMacro(const Token* tk, bool updatePeekToken = true);" should be removed.

Those kinds of functions just replace some piece of text in the buffer, no need to update peek token, the caller can explicitly call PeekToken or GetToken functions

EDIT1: when using git blame, I found that the second parameter of this function was added in

--- Code: ---* * cc_branch: applied patch with rewritten function-like macro handle (v5)
git-svn-id: http://svn.code.sf.net/p/codeblocks/code/branches/codecompletion_refactoring@6436  
--- End code ---
It is used to expand function-like macro usage.

EDIT2, the second parameter of ReplaceFunctionLikeMacro() as added in

--- Code: ---* * cc_branch: improved function-like macro parsing, a little improved for avoid the endless loop

git-svn-id: http://svn.code.sf.net/p/codeblocks/code/branches/codecompletion_refactoring@6691

--- End code ---


EDIT3
It looks like all the function call with second parameter value == true is used for handling macro expansion. (HandleMacro)
Here is my analysis:
Rev6436
The first place of ReplaceBufferForReparse() with second parameter value == true:

--- Code: ---+wxString Tokenizer::GetActualContextForMacro(Token* tk)
+{
+    if (!tk)
+        return wxEmptyString;
+
+    // 1. break the args into substring with "," and store them in normals
+    wxArrayString normalArgs;
+    if (!tk->m_Args.IsEmpty())
+    {
+        ReplaceBufferForReparse(tk->m_Args);
+        SpliteArguments(normalArgs);
+    }
--- End code ---

Here, not sure why this need to run peek again?
Since ReplaceBufferForReparse() just put a new piece of text in the buffer(a macro definition's formal parameter), and move back the m_TokenIndex a bit, what is the reason to internally call PeekToken() function?

The second place of ReplaceBufferForReparse() with second parameter value == true:


--- Code: ---+        TRACE(_T("HandleMacro() : Adding token '%s' (peek='%s')"), tk->m_Name.wx_str(), peek.wx_str());
+        DoAddToken(tkMacro, tk->m_Name, m_Tokenizer.GetLineNumber(), 0, 0, peek);
+        m_Tokenizer.ReplaceBufferForReparse(m_Tokenizer.GetActualContextForMacro(tk));
--- End code ---

Which is inside the ParserThread::HandleMacro() function (handling macro usage) body, but I still don't see a forced PeekToken() call is necessary.
---------------
Rev6691
First place of the ReplaceMacroActualContext() function call with second parameter value == true

--- Code: ---void ParserThread::HandleMacro(int id, const wxString &peek)
{
    Token* tk = m_TokensTree->at(id);
    if (tk)
    {
        TRACE(_T("HandleMacro() : Adding token '%s' (peek='%s')"), tk->m_Name.wx_str(), peek.wx_str());
        DoAddToken(tkMacro, tk->m_Name, m_Tokenizer.GetLineNumber(), 0, 0, peek);

        if (m_Options.parseComplexMacros)
            m_Tokenizer.ReplaceMacroActualContext(tk);
    }
}
--- End code ---
I think it is not necessary to run PeekToken() inside ReplaceMacroActualContext().

Huki:

--- Quote from: ollydbg on September 17, 2014, 05:45:00 am ---I'm thinking that the code:
[...]
Should be changed to


--- Code: ---bool Tokenizer::ReplaceBufferText(const wxString& target)
{
    ...
    ...
    // Fix token index
    m_TokenIndex -= bufferLen;

    // Reset undo token
    m_SavedTokenIndex   = m_UndoTokenIndex = m_TokenIndex;
    m_SavedLineNumber   = m_UndoLineNumber = m_LineNumber;
    m_SavedNestingLevel = m_UndoNestLevel  = m_NestLevel;

    // Update the peek token
    if (m_PeekAvailable)
        m_PeekAvailable = false;


    return true;
}

--- End code ---

--- End quote ---

I agree.. in the current code, all internal ReplaceBufferText calls (ie, called inside Tokenizer.cpp) have updatePeek set to false, and external ReplaceBufferText calls have it set to true. I checked all the external calls (there were just a few of them, and all of them in parserthread.cpp), and we always do a GetToken() after the ReplaceBufferText(). So I think it's safe to remove this parameter.

ollydbg:

--- Quote from: Huki on September 18, 2014, 12:56:11 pm ---...
...
I agree.. in the current code, all internal ReplaceBufferText calls (ie, called inside Tokenizer.cpp) have updatePeek set to false, and external ReplaceBufferText calls have it set to true. I checked all the external calls (there were just a few of them, and all of them in parserthread.cpp), and we always do a GetToken() after the ReplaceBufferText(). So I think it's safe to remove this parameter.


--- End quote ---
Thanks, done in trunk(rev9917 and rev9918).

ollydbg:
When try to fix the issue reported here: Re: The 02 March 2014 build (9673.) is out.
I found that all the identifier like token should be checked for macro replacement, inside the DoGetToken(), other wise

--- Code: ---void ParserThread::SkipBlock()
{
    // need to force the tokenizer _not_ skip anything
    // or else default values for template params would cause us to miss everything (because of the '=' symbol)
    TokenizerState oldState = m_Tokenizer.GetState();
    m_Tokenizer.SetState(tsSkipNone);

    // skip tokens until we reach }
    // block nesting is taken into consideration too ;)

    // this is the nesting level we start at
    // we subtract 1 because we 're already inside the block
    // (since we 've read the {)
    unsigned int level = m_Tokenizer.GetNestingLevel() - 1;
    while (IS_ALIVE)
    {
        wxString token = m_Tokenizer.GetToken();
        if (token.IsEmpty())
            break; // eof

        // if we reach the initial nesting level, we are done
        if (level == m_Tokenizer.GetNestingLevel())
            break;
    }

    // reset tokenizer's functionality
    m_Tokenizer.SetState(oldState);
}

--- End code ---
This function don't expand macro expansion if token is a macro usage.

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version