Author Topic: Several improvements to Code Completion plugin  (Read 160743 times)

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #135 on: September 05, 2014, 04:55:57 pm »
@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. :)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #136 on: September 17, 2014, 05:45:00 am »
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;
}

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;
}

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  
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


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);
+    }

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));

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);
    }
}
I think it is not necessary to run PeekToken() inside ReplaceMacroActualContext().

« Last Edit: September 17, 2014, 09:48:32 am by ollydbg »
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #137 on: September 18, 2014, 12:56:11 pm »
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;
}

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.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #138 on: September 18, 2014, 04:56:46 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.

Thanks, done in trunk(rev9917 and rev9918).
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #139 on: October 06, 2014, 06:58:48 am »
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);
}
This function don't expand macro expansion if token is a macro usage.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #140 on: April 11, 2015, 04:30:36 pm »
Any news on my cc_enum_values.patch? I'm attaching it again and the supported test cases in cc_enum_test.patch.
- Uses expression solver to calculate the enum value, expand macro assignments.
- Supports enum assignment to a previous enum (checks under the correct parent).
- If the expression cannot be evaluated (eg, unknown macro), leave it and the next enums blank and don't reset it to zero.
Hi, Huki, this cause some parsing error, see: Code Completion problem with some wx classes.
Any idea how to fix this?
Thanks.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.