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

Several improvements to Code Completion plugin

<< < (9/29) > >>

Huki:

--- Quote from: oBFusCATed on September 18, 2013, 01:02:25 am ---@huki: Can I ask you to provide a patch which adds you to the "help -> about -> thanks to" dialog?

--- End quote ---
Sure, here you go:


--- Code: ---Index: src/src/dlgabout.cpp
===================================================================
--- src/src/dlgabout.cpp (revision 9271)
+++ src/src/dlgabout.cpp (working copy)
@@ -139,6 +139,7 @@
         "Sylvain Prat        : Initial MSVC workspace and project importers\n"
         "Chris Raschko       : Design of the 3D logo for Code::Blocks\n"
         "J.A. Ortega         : 3D Icon based on the above\n"
+        "Huki                : Patches for Code Completion plugin\n"
         "\n"
         "All contributors that provided patches.\n"
         "The wxWidgets project (http://www.wxwidgets.org).\n"

--- End code ---

ollydbg:

--- Quote from: Huki on September 19, 2013, 12:52:22 am ---@ollydbg: Thanks! I see one fix related to '==' sign hasn't made it through, in ReadParantheses(). See the code below with my comment.

--- Code: ---        case _T('='):
            {
                if (*(p - 1) <= _T(' '))
                {
                    *p = _T('=');
                    // Don't add a space after '=' sign, in case another '=' follows it
                    // (see how the 'else' block below works).
                    //*++p = _T(' ');
                    ++p;
                }
                else
                {
                    switch (*(p - 1))
                    {
                    case _T('='):
                    case _T('!'):
                    case _T('>'):
                    case _T('<'):
                        {
                            *p = _T('=');
                            *++p = _T(' ');
                            ++p;
                        }
                        break;

                    default:
                        {
                            *p = _T(' ');
                            *++p = _T('=');
                            *++p = _T(' ');
                            ++p;
                        }
                        break;
                    }
                }
            }
            break;

        [...]

        // '<', '>' and '!' are handled here.
        default:
            {
                *p = ch;
                ++p;
            }
            break;

--- End code ---

And here is the patch:

--- Code: (Fixes space between '==') ---Index: src/plugins/codecompletion/parser/tokenizer.cpp
===================================================================
--- src/plugins/codecompletion/parser/tokenizer.cpp (revision 9271)
+++ src/plugins/codecompletion/parser/tokenizer.cpp (working copy)
@@ -713,7 +718,6 @@
                 if (*(p - 1) <= _T(' '))
                 {
                     *p = _T('=');
-                    *++p = _T(' ');
                     ++p;
                 }
                 else

--- End code ---


--- End quote ---
FYI: Morten has applied this patch (rev9351), thanks.

Huki:
Ok.. about the issue with ReplaceBufferForReparse(), I think it will be more clear with an example.
The goal is to get UngetToken() to work reliably when macro expansions are involved.

As an example, we have this code:

--- Code: ---#define MYMACRO X=0 /*Alternatively, user might have a replacement rule for MYMACRO*/
int X;
MYMACRO;
X = 1;

--- End code ---

^ denotes the token index.
_ underlines the current token string.
EOL char is ignored for our example.

CASE I: when ReplaceBufferForReparse() is called in GetToken() / used directly before calling GetToken().
GetToken() Begin:

--- Code: ---            int X;MYMACRO;X = 1;
Token:           _^
 Undo:           ^

--- End code ---
Step 1: Update undo token and get next token.

--- Code: ---            int X;MYMACRO;X = 1;
Token:            _______^
 Undo:            ^

--- End code ---
Step 2: ReplaceBufferForReparse(): Replace the macro and reset token index.
BUG HERE: Undo token is still pointing to bad location!

--- Code: ---            int X;MYMAX=0;X = 1;
Token:                ^
 Undo:            ^

--- End code ---
FIX: Move the undo index to correct location.

--- Code: ---            int X;MYMAX=0;X = 1;
Token:                ^
 Undo:                ^

--- End code ---
Step 3: Get the next token now after replace.

--- Code: ---            int X;MYMAX=0;X = 1;
Token:                _^
 Undo:                ^

--- End code ---


CASE II: when ReplaceBufferForReparse() is called in PeekToken(), we have an additional problem.
PeekToken() Begin:
Step 1: Save the token index.

--- Code: ---            int X;MYMACRO;X = 1;
Token:           _^
 Save:            ^

--- End code ---

Step 2: DoGetToken():
 - Get next token.

--- Code: ---            int X;MYMACRO;X = 1;
Token:            _______^
 Save:            ^

--- End code ---
- Replace the macro and reset token index.

--- Code: ---            int X;MYMAX=0;X = 1;
Token:                ^
 Save:            ^

--- End code ---
- Get the next token now after replace.

--- Code: ---            int X;MYMAX=0;X = 1;
Token:                _^
 Save:            ^

--- End code ---

Step 3: Assign Token to Peek, then restore Token from Save.
BUG HERE: Save is still pointing to bad location!

--- Code: ---            int X;MYMAX=0;X = 1;
 Peek:                _^
Token:           _^

--- End code ---
FIX: Reset the token index to correct location.

--- Code: ---            int X;MYMAX=0;X = 1;
 Peek:                _^
Token:           _    ^

--- End code ---


Here is an updated patch with fixes for both cases.

--- Code: (CC: Reliable working of UngetToken() when macro expansion is involved) ---Index: src/plugins/codecompletion/parser/tokenizer.cpp
===================================================================
--- src/plugins/codecompletion/parser/tokenizer.cpp (revision 9271)
+++ src/plugins/codecompletion/parser/tokenizer.cpp (working copy)
@@ -1135,6 +1146,8 @@
         unsigned int savedLineNumber = m_LineNumber;
         unsigned int savedNestLevel  = m_NestLevel;
 
+        int savedReplaceCount = m_IsReplaceParsing ? m_RepeatReplaceCount : -1;
+
         if (SkipUnwanted())
             m_PeekToken = DoGetToken();
         else
@@ -1144,9 +1157,20 @@
         m_PeekLineNumber             = m_LineNumber;
         m_PeekNestLevel              = m_NestLevel;
 
-        m_TokenIndex                 = savedTokenIndex;
-        m_LineNumber                 = savedLineNumber;
-        m_NestLevel                  = savedNestLevel;
+        // Check whether a ReplaceBufferForReparse() was done in DoGetToken().
+        // We assume m_Undo... have already been reset in ReplaceBufferForReparse().
+        if (m_IsReplaceParsing && savedReplaceCount != (int)m_RepeatReplaceCount)
+        {
+            m_TokenIndex             = m_UndoTokenIndex;
+            m_LineNumber             = m_UndoLineNumber;
+            m_NestLevel              = m_UndoNestLevel;
+        }
+        else
+        {
+            m_TokenIndex             = savedTokenIndex;
+            m_LineNumber             = savedLineNumber;
+            m_NestLevel              = savedNestLevel;
+        }
     }
 
     return m_PeekToken;
@@ -1818,6 +1871,11 @@
     // Fix token index
     m_TokenIndex -= bufferLen;
 
+    // Fix undo position
+    m_UndoTokenIndex = m_TokenIndex;
+    m_UndoLineNumber = m_LineNumber;
+    m_UndoNestLevel = m_NestLevel;
+
     // Update the peek token
     if (m_PeekAvailable && updatePeekToken)
     {

--- End code ---

ollydbg:

--- Quote from: Huki on September 21, 2013, 09:29:23 pm ---Ok.. about the issue with ReplaceBufferForReparse(), I think it will be more clear with an example.
The goal is to get UngetToken() to work reliably when macro expansions are involved.
...

--- End quote ---
Hi, thanks for the explanation, your patch is OK from my review. :D

Besides that, I think an improved UngetToken() can be:

--- Code: ---/* peek is always available when we run UngetToken() once, actually the m_TokenIndex is moved
 * backward one step. Note that the m_UndoTokenIndex value is not updated in this function, which
 * means if you are not allowed to run this function twice.
 */
void Tokenizer::UngetToken()
{
    if(m_TokenIndex == m_UndoTokenIndex) //this means we have already run a UngetToken() before.
        return;

    m_PeekTokenIndex = m_TokenIndex;
    m_PeekLineNumber = m_LineNumber;
    m_PeekNestLevel  = m_NestLevel;
    m_TokenIndex     = m_UndoTokenIndex;
    m_LineNumber     = m_UndoLineNumber;
    m_NestLevel      = m_UndoNestLevel;
    m_PeekToken      = m_Token;
    m_PeekAvailable  = true;
}

--- End code ---
We only allow the user run UngetToken() once,  not twice, right?

ollydbg:
@Huki, I commit a modified patch: Code: (CC: Reliable working of UngetToken() when macro expansion is involved) in rev9369. Thanks. There are still some patches left for testing, no hurry. :)

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version