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

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #90 on: June 18, 2014, 09:10:27 pm »
Thanks for the commit.
Two patches for build targets parsing (i.e., project defined macros):

1) When a virtual target is selected, collect the defines for all child targets. Eg., in the CB project, if the "All" virtual target is selected we should collect defines from all sub-targets (scintilla, etc).
Code
Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -2342,6 +2367,18 @@
         for (size_t i = 0; i < targetOpts.GetCount(); ++i)
             opts.Add(targetOpts[i]);
     }
+    // In case of virtual targets, collect the defines from all child targets.
+    wxArrayString targets = project->GetExpandedVirtualBuildTargetGroup(project->GetActiveBuildTarget());
+    for (size_t i = 0; i < targets.GetCount(); ++i)
+    {
+        target = project->GetBuildTarget(targets[i]);
+        if (target != NULL)
+        {
+            wxArrayString targetOpts = target->GetCompilerOptions();
+            for (size_t j = 0; j < targetOpts.GetCount(); ++j)
+                opts.Add(targetOpts[j]);
+        }
+    }
 
     for (size_t i = 0; i < opts.GetCount(); ++i)
     {

2) When the build target is changed, trigger a whole project reparse.
Code
Index: src/plugins/codecompletion/codecompletion.cpp
===================================================================
--- src/plugins/codecompletion/codecompletion.cpp (revision 9271)
+++ src/plugins/codecompletion/codecompletion.cpp (working copy)
@@ -647,6 +695,8 @@
 
     pm->RegisterEventSink(cbEVT_WORKSPACE_CHANGED,    new cbEventFunctor<CodeCompletion, CodeBlocksEvent>(this, &CodeCompletion::OnWorkspaceChanged));
 
+    pm->RegisterEventSink(cbEVT_BUILDTARGET_SELECTED, new cbEventFunctor<CodeCompletion, CodeBlocksEvent>(this, &CodeCompletion::OnBuildTargetSelected));
+
     pm->RegisterEventSink(cbEVT_PROJECT_ACTIVATE,     new cbEventFunctor<CodeCompletion, CodeBlocksEvent>(this, &CodeCompletion::OnProjectActivated));
     pm->RegisterEventSink(cbEVT_PROJECT_CLOSE,        new cbEventFunctor<CodeCompletion, CodeBlocksEvent>(this, &CodeCompletion::OnProjectClosed));
     pm->RegisterEventSink(cbEVT_PROJECT_SAVE,         new cbEventFunctor<CodeCompletion, CodeBlocksEvent>(this, &CodeCompletion::OnProjectSaved));
@@ -2552,6 +2609,29 @@
     event.Skip();
 }
 
+void CodeCompletion::OnBuildTargetSelected(CodeBlocksEvent& event)
+{
+    if (!ProjectManager::IsBusy() && IsAttached() && m_InitDone)
+    {
+        cbProject* project = Manager::Get()->GetProjectManager()->GetActiveProject();
+        if (project)
+        {
+            TRACE(_T("CodeCompletion::OnBuildTargetSelected(): Reparsing entire project."));
+
+            ReparsingMap::iterator it = m_ReparsingMap.find(project);
+            if (it != m_ReparsingMap.end())
+                m_ReparsingMap.erase(it);
+            if (m_NativeParser.DeleteParser(project))
+                m_NativeParser.CreateParser(project);
+
+            m_AutocompNameIdx.clear();
+            m_LastAutocompIndex = -1;
+        }
+    }
+
+    event.Skip();
+}
+
 void CodeCompletion::OnProjectActivated(CodeBlocksEvent& event)
 {
     // The Class browser shouldn't be updated if we're in the middle of loading/closing
Index: src/plugins/codecompletion/codecompletion.h
===================================================================
--- src/plugins/codecompletion/codecompletion.h (revision 9271)
+++ src/plugins/codecompletion/codecompletion.h (working copy)
@@ -196,6 +200,7 @@
     void OnProjectFileAdded(CodeBlocksEvent& event);
     void OnProjectFileRemoved(CodeBlocksEvent& event);
     void OnProjectFileChanged(CodeBlocksEvent& event);
+    void OnBuildTargetSelected(CodeBlocksEvent& event);
     /** SDK editor related events */
     void OnEditorSaveOrModified(CodeBlocksEvent& event);
     void OnEditorOpen(CodeBlocksEvent& event);

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #91 on: June 21, 2014, 04:01:26 am »
The patch is still against old svn revision(92xx), we are currently in rev 98xx, there are around 600 commits.  :)
It looks like
Code
   m_AutocompNameIdx.clear();
            m_LastAutocompIndex = -1;
are totally removed in commit rev9694(CC management commits serials by alpha), I'm not sure it is simple remove the above two lines from your patch, still need time to test. :)
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 #92 on: June 21, 2014, 05:10:40 am »
The patch is still against old svn revision(92xx), we are currently in rev 98xx, there are around 600 commits.  :)
It looks like
Code
   m_AutocompNameIdx.clear();
            m_LastAutocompIndex = -1;
are totally removed in commit rev9694(CC management commits serials by alpha), I'm not sure it is simple remove the above two lines from your patch, still need time to test. :)
Oh, those two lines can be safely removed.. I should have removed them myself but forgot.
Initially I took code from CodeCompletion::OnCurrentProjectReparse() which contained the Autocomp lines. But this code parses the current project (i.e., project of active editor). The goal is to reparse the active project (for which build target was changed), so we should do similar to CodeCompletion::OnProjectSavedTimer():
- Get the active project.
- Search for and remove project from m_ReparsingMap.
- Delete project's parser.
- Create project parser.

So the Autocomp lines are not required.
« Last Edit: June 21, 2014, 05:12:48 am by Huki »

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re:
« Reply #93 on: June 21, 2014, 07:12:54 am »
Still, please always try to provide patches against trunk HEAD. Everything else is error prone.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #94 on: June 21, 2014, 01:28:07 pm »
@Huki.
About parsing the target in a project. See an example.
A project have two targets: TargetA, TargetB.
TargetA is the active target, which contains two cpp file (A1.cpp, A2.cpp), TargetB contains two cpp file (B1.cpp, B2.cpp).

Current method:  parse all the four files with the preprecessor directive from TargetA, see bool NativeParser::DoFullParsing(cbProject* project, ParserBase* parser).

Is this OK? 

Or, is it better to only parse the files(A1.cpp, A2.cpp) belong to the active target? I don't have an idea.


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 #95 on: June 21, 2014, 04:20:47 pm »
@Huki.
About parsing the target in a project. See an example.
A project have two targets: TargetA, TargetB.
TargetA is the active target, which contains two cpp file (A1.cpp, A2.cpp), TargetB contains two cpp file (B1.cpp, B2.cpp).

Current method:  parse all the four files with the preprecessor directive from TargetA, see bool NativeParser::DoFullParsing(cbProject* project, ParserBase* parser).

Is this OK?  

Or, is it better to only parse the files(A1.cpp, A2.cpp) belong to the active target? I don't have an idea.

Hmm, it would be a good optimization indeed. If a certain file is not going to be compiled then there isn't much point in parsing it. It would mean for example, if I have selected the Code Completion target, then with code such as project->GetBuildTarget(), I can only use "go to declaration".. "go to implementation" would fail. Anyway, I think it's the expected behavior since we are not supposed to modify the sdk code when in CC target, as per our example.

One caveat is, what happens when a non-parsed file is activated in the editor?
- If the file does not belong to any project, it is added to a special parser for standalone files and parsed.
- If the file belongs to the current project nothing would happen (currently it is assumed that a file belonging to a project will always be parsed, so this case isn't covered).
Do you think this behavior needs to be changed? i.e., If a file belongs to the project but not part of the active target (and hence not parsed), should it be added to the standalone parser file's list if it is opened in the editor?

Anyway, if we want to try this optimization it appears to be simple: in DoFullParsing() we can replace project->GetFilesList() with:
1) project->GetBuildTarget(active_target)
2) Check if it's a real or virtual target
2a) If real, target->GetFilesList()
2b) If virtual, do according to the patch I submitted for collecting defines.


PS: I'm setting up git as we speak.. :)
« Last Edit: June 21, 2014, 04:22:43 pm by Huki »

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #96 on: June 21, 2014, 05:05:30 pm »
Hmm, it would be a good optimization indeed. If a certain file is not going to be compiled then there isn't much point in parsing it. It would mean for example, if I have selected the Code Completion target, then with code such as project->GetBuildTarget(), I can only use "go to declaration".. "go to implementation" would fail.
Yes, "go to implementation" would fail, if we only parse files belong to a single target(like Code Completion plugin target).

Quote
Anyway, I think it's the expected behavior since we are not supposed to modify the sdk code when in CC target, as per our example.
But some user may ask that ""go to implementation" should still work. It's a good feature if we can quickly find the implementation.
Note, we already have an option: one parser for the whole workspace, which means one token tree holds all the tokens from every file in the workspace (a workspace may containing many projects), I can guess the reason is that we want a whole declaration/implementation cross reference between all the projects.


Quote
One caveat is, what happens when a non-parsed file is activated in the editor?
- If the file does not belong to any project, it is added to a special parser for standalone files and parsed.
Yes, those files will be put in a special parser named "NON" parser.

Quote
- If the file belongs to the current project nothing would happen (currently it is assumed that a file belonging to a project will always be parsed, so this case isn't covered).
Yes, if the new opened file is in the token tree, it won't be parsed, it may switch the parser if necessary.


Quote
Do you think this behavior needs to be changed? i.e., If a file belongs to the project but not part of the active target (and hence not parsed), should it be added to the standalone parser file's list if it is opened in the editor?
I personally still need the cross reference feature.
I'd like to see other guys option.
I have a new idea: first parse the files belong to the active target, then other files in the same project. If the active target changed, reparse only the files belong to the new target.


Quote
Anyway, if we want to try this optimization it appears to be simple: in DoFullParsing() we can replace project->GetFilesList() with:
1) project->GetBuildTarget(active_target)
2) Check if it's a real or virtual target
2a) If real, target->GetFilesList()
2b) If virtual, do according to the patch I submitted for collecting defines.
Yes, this is the way if we want to implement the "per target parsing".
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 #97 on: June 21, 2014, 07:41:34 pm »
I personally still need the cross reference feature.
I'd like to see other guys option.
I have a new idea: first parse the files belong to the active target, then other files in the same project. If the active target changed, reparse only the files belong to the new target.
If the cross reference feature is needed, then I think it's better to keep the current system of full project reparse. Your new idea is almost good enough, but it assumes that the change in project macros list only affects the new target. Most of the time this is not true, eg., switching from Scintilla target to CC target: the macros used by scintilla target (such as SCI_LEXER) will be removed, so we need to reparse the old target too.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #98 on: June 23, 2014, 03:53:44 am »
I personally still need the cross reference feature.
I'd like to see other guys option.
I have a new idea: first parse the files belong to the active target, then other files in the same project. If the active target changed, reparse only the files belong to the new target.
If the cross reference feature is needed, then I think it's better to keep the current system of full project reparse. Your new idea is almost good enough, but it assumes that the change in project macros list only affects the new target. Most of the time this is not true, eg., switching from Scintilla target to CC target: the macros used by scintilla target (such as SCI_LEXER) will be removed, so we need to reparse the old target too.

Agree, in some project, files were shared by several targets, this way, if a file is already parsed in targetA, it will never be parsed in targetB.
So, your two patches in
Thanks for the commit.
Two patches for build targets parsing (i.e., project defined macros):
...
are not needed, right? Unless we find a better way.
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 #99 on: June 23, 2014, 07:05:15 am »
So, your two patches in
Thanks for the commit.
Two patches for build targets parsing (i.e., project defined macros):
...
are not needed, right? Unless we find a better way.
Hmm, the first patch is needed IMO (i.e, "When a virtual target is selected, collect the defines for all child targets."). In the CB project for example, check the parts covered by SCI_LEXER macro in ScintillaBase.cxx (sdk\wxscintilla\src\scintilla\src\ScintillaBase.cxx). With the "All" virtual target, SCI_LEXER won't be defined, so the code covered by it won't be parsed by CC. I think when a virtual target is chosen, it is better to collect the defines set for the virtual target, and also all the defines set for its child targets.

The second patch is more of a performance concern. Most projects have a Debug and Release target (where a DEBUG macro is defined for the Debug target). In this case when the target is changed, the user expects that the DEBUG code is enabled or disabled accordingly, which means the full project needs to be reparsed with the new macro state. Personally I don't mind the time taken for full project reparse, so I'd suggest committing the second patch too. :)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #100 on: June 23, 2014, 04:19:02 pm »
Hmm, the first patch is needed IMO (i.e, "When a virtual target is selected, collect the defines for all child targets."). In the CB project for example, check the parts covered by SCI_LEXER macro in ScintillaBase.cxx (sdk\wxscintilla\src\scintilla\src\ScintillaBase.cxx). With the "All" virtual target, SCI_LEXER won't be defined, so the code covered by it won't be parsed by CC. I think when a virtual target is chosen, it is better to collect the defines set for the virtual target, and also all the defines set for its child targets.
OK, I commit the first patch in r9824. Thanks.

Quote
The second patch is more of a performance concern. Most projects have a Debug and Release target (where a DEBUG macro is defined for the Debug target). In this case when the target is changed, the user expects that the DEBUG code is enabled or disabled accordingly, which means the full project needs to be reparsed with the new macro state. Personally I don't mind the time taken for full project reparse, so I'd suggest committing the second patch too. :)
I even thought that we can switch the macro definition set when parsing different targets' source files, not sure this is doable. I do care about the performance issue. When developing C::B, I ways switch the build target from time to time. (I use thread search plugin, I need the search only limited to the current active target). So, I personally don't like the idea that switching the target cause a reparse. But I would like to see all the comments about this.

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 #101 on: June 24, 2014, 08:27:49 pm »
The second patch is more of a performance concern. Most projects have a Debug and Release target (where a DEBUG macro is defined for the Debug target). In this case when the target is changed, the user expects that the DEBUG code is enabled or disabled accordingly, which means the full project needs to be reparsed with the new macro state. Personally I don't mind the time taken for full project reparse, so I'd suggest committing the second patch too. :)
I even thought that we can switch the macro definition set when parsing different targets' source files, not sure this is doable. I do care about the performance issue. When developing C::B, I ways switch the build target from time to time. (I use thread search plugin, I need the search only limited to the current active target). So, I personally don't like the idea that switching the target cause a reparse. But I would like to see all the comments about this.
I understand about that.. I'm going to keep this feature aside for now, we'll see if we are able to find a better solution in the future.


Btw, I think you remember our fix for reliable UngetToken() behavior (see here for the original discussion). To recap, in PeekToken(): we are checking the value of m_IsReplaceParsing before and after DoGetToken() to see if a ReplaceBufferText() was done, and if it was done, we should reset the savedTokenIndex, etc.

Now, this way of checking m_IsReplaceParsing can fail in some cases:
1) We are already in replace mode (eg, m_IsReplaceParsing == 1) before DoGetToken(), and inside DoGetToken() m_IsReplaceParsing is reset to zero and a new macro replacement is started (once again m_IsReplaceParsing == 1). No change will be detected and we will fail to reset the savedToken position.
2) A preprocessor condition is hit in SkipUnwanted() and we run CalcConditionExpression(). Now, CalcConditionExpression() will not only expand the macro, but also parse the entire expanded macro and compute the result. By the time we exit SkipUnwanted(), m_IsReplaceParsing is still non-zero, but we are already at the end of the expanded macro. Now, the DoGetToken() will reset m_IsReplaceParsing to zero, and once again no change will be detected. i.e.,
Code
    savedReplaceParsing = m_IsReplaceParsing; // m_IsReplaceParsing == 0
   
    if (SkipUnwanted()) // ReplaceBufferText() was run and expanded macro parsed, m_IsReplaceParsing == 1
        DoGetToken(); // out of replace buffer, m_IsReplaceParsing reset to 0

    // check if ReplaceBufferText() was run
    if (savedReplaceParsing < m_IsReplaceParsing) // condition failed
        [...]

Yesterday I was unlucky enough to hit both these cases 1) and 2) in the same code, so we definitely need some improvement.. ;)

I think the proper way of doing it is to make the saved___ variables class members of Tokenizer, so they can be accessed from ReplaceBufferText() and reset when required (just like we are resetting the m_Undo___ variables). We also need to reset Undo and Saved positions to the proper place after CalcConditionExpression() (because as I've said before it's not only expanding the macro but also parsing it fully, so we need to reset right after the preprocessor line has ended).

I'll post the patch with these changes and some test cases below.

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #102 on: June 24, 2014, 09:34:00 pm »
I'm attaching the patch below: (git patch against very recently synced master)
- Moved local variables saved___ to Tokenizer class, m_Saved___.
- Reset m_Undo and m_Saved vars to proper position in ReplaceBufferText().
- Reset m_Undo and m_Saved vars to proper position in CalcConditionExpression().
- Added missing line in GetPreprocessorType().
- Reverted your changes in UnGetToken() and left a note.

Now for some test cases:
1) When ReplaceBufferText() is done in DoGetToken(), (the usual case):
Code
Our code:
#define MYMACRO X=0
int X;
MYMACRO;
X = 1;

PeekToken() Begin:
            int X;MYMACRO;X=1;
Token:           _^
Undo :           ^
Peek :

Update m_SavedTokenIndex and run DoGetToken():
            int X;MYMACRO;X=1;
Token:           _       ^
Save :            ^
Undo :           ^
Peek :

ReplaceBufferText() and reset m_TokenIndex:
            int X;MYMAX=0;X=1;
Token:           _    ^
Save :            ^
Undo :           ^
Peek :

(Added by patch) Reset m_Undo and m_Save position:
            int X;MYMAX=0;X=1;
Token:           _    ^
Save :                ^
Undo :                ^
Peek :

Get the next token:
(Note that the string will be saved in m_PeekToken,
 m_Token will be untouched)
            int X;MYMAX=0;X=1;
Token:           _     ^
Save :                ^
Undo :                ^
Peek :                _

PeekToken() End: update m_PeekTokenIndex and restore m_TokenIndex from m_SaveTokenIndex:
            int X;MYMAX=0;X=1;
Token:           _    ^
Undo :                ^
Peek :                _^

Now, user calls UnGetToken(): (m_PeekAvailable becomes true)
(Note that even though m_TokenIndex == m_UndoTokenIndex, we should allow UnGetToken()
 to run anyway. Of course, we should only call it once after each GetToken())
            int X;MYMAX=0;X=1;
Token:                ^
Undo :                ^
Peek :           _    ^

GetToken() again: (take from peek and reset m_PeekAvailable to false)
            int X;MYMAX=0;X=1;
Token:           _    ^
Undo :                ^
Peek :

2) When CalcConditionExpression() is called in SkipUnwanted() immediately before DoGetToken():
Note that I have modified the example code to remove the pointer (it was originally "typedef RGBQUAD *LPRGBQUAD;"). But I don't think the current CC typedef support works well with pointers (and I'm yet to submit my patch for it).

2a) A simple case when there is no macro replacement involved:
Code
  // just some code :)
  typedef struct tagRGBQUAD {
    char rgbBlue;
    char rgbGreen;
    char rgbRed;
    char rgbReserved;
  } RGBQUAD;

#if 1
  typedef RGBQUAD LPRGBQUAD;
#endif

PeekToken() Begin:
        RGBQUAD;#if 1 typedef RGBQUAD LPRGBQUAD;
Token          _^
Undo           ^
Peek

PeekToken(): SkipUnwanted():
Note: It will run CalcConditionExpression(), evaluate the result to
 true and skip to the first character after the condition expression.
        RGBQUAD;#if 1 typedef RGBQUAD LPRGBQUAD;
Token          _      ^
Save            ^
Undo           ^
Peek

PeekToken(): SkipUnwanted(): CalcConditionExpression() End:
We reset Save and Undo:
        RGBQUAD;#if 1 typedef RGBQUAD LPRGBQUAD;
Token          _      ^
Save                  ^
Undo                  ^
Peek

PeekToken(): DoGetToken():
        RGBQUAD;#if 1 typedef RGBQUAD LPRGBQUAD;
Token          _             ^
Save                  ^
Undo                  ^
Peek                  _______

PeekToken() End: Reset Token from Save:
        RGBQUAD;#if 1 typedef RGBQUAD LPRGBQUAD;
Token          _      ^
Undo                  ^
Peek                  _______^

Now user runs UnGetToken(): m_PeekAvailable becomes true:
Note again as in our first example, TokenIndex == UndoIndex,
 but we should run UnGetToken() anyway.
        RGBQUAD;#if 1 typedef RGBQUAD LPRGBQUAD;
Token                 ^
Undo                  ^
Peek           _      ^

GetToken() again: Use peek and reset m_PeekAvailable:
        RGBQUAD;#if 1 typedef RGBQUAD LPRGBQUAD;
Token          _      ^
Undo                  ^
Peek

Of course, in this simple case there was no macro replacement, so although resetting the undo and save tokens was good, it was not really needed for proper parsing. In case of macro replacement it becomes necessary.

« Last Edit: June 24, 2014, 10:13:57 pm by Huki »

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Several improvements to Code Completion plugin
« Reply #103 on: June 24, 2014, 10:05:37 pm »
2b) A (lot) more complex case of previous example, taken from the mingw-builds winapi code which gave me trouble yesterday:
Our code:
Code
#define WINAPI_PARTITION_DESKTOP 0x1
#define WINAPI_PARTITION_APP     0x2

#define WINAPI_FAMILY_APP WINAPI_PARTITION_APP
#define WINAPI_FAMILY_DESKTOP_APP (WINAPI_PARTITION_DESKTOP \
   | WINAPI_PARTITION_APP)

/* WINAPI_FAMILY can be either desktop + App, or App.  */
#ifndef WINAPI_FAMILY
#define WINAPI_FAMILY WINAPI_FAMILY_DESKTOP_APP
#endif

#define WINAPI_FAMILY_PARTITION(v) ((WINAPI_FAMILY & v) == v)
#define WINAPI_FAMILY_ONE_PARTITION(vset, v) ((WINAPI_FAMILY & vset) == v)

  typedef struct tagRGBQUAD {
    char rgbBlue;
    char rgbGreen;
    char rgbRed;
    char rgbReserved;
  } RGBQUAD;

#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)
  typedef RGBQUAD LPRGBQUAD; // removed *
#endif

Parsing:
Code
PeekToken() Begin:
                                                   RGBQUAD;#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                                     _^
Undo                                                      ^
Peek

PeekToken(): SkipUnwanted(): GetPreprocessorType():
                                                   RGBQUAD;#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                                     _   ^
Save                                                       ^
Undo                                                      ^
Peek

PeekToken(): SkipUnwanted(): CalcConditionExpression():
Note: This will expand the macro using ReplaceBufferText() which will
 already reset Undo and Save positions. But is it enough?
                                                   RGBQUAD;#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                                     _                           ^
Save                                                       ^
Undo                                                      ^
Peek
                                        ((WINAPI_FAMILY & WINAPI_PARTITION_DESKTOP) == WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                   ^                 ;
Save                                    ^
Undo                                    ^
Peek
                              WINAPI_FAMILY_DESKTOP_APP & WINAPI_PARTITION_DESKTOP) == WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                         ^                           ;
Save                          ^
Undo                          ^
Peek
        (WINAPI_PARTITION_DESKTOP|WINAPI_PARTITION_APP) & WINAPI_PARTITION_DESKTOP) == WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token   ^                                                 ;
Save    ^
Undo    ^
Peek

PeekToken(): SkipUnwanted(): CalcConditionExpression():
Now the entire expanded macro will be parsed, so we are left at the end of it:
        (WINAPI_PARTITION_DESKTOP|WINAPI_PARTITION_APP) & WINAPI_PARTITION_DESKTOP) == WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                                     ;                                                     ^
Save    ^
Undo    ^
Peek

PeekToken(): SkipUnwanted(): CalcConditionExpression(): End:
The condition macro has lost its context (we lost the #if),
 so we must reset undo and save to the end of the macro statement.
        (WINAPI_PARTITION_DESKTOP|WINAPI_PARTITION_APP) & WINAPI_PARTITION_DESKTOP) == WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                                     ;                                                     ^
Save                                                                                                            ^
Undo                                                                                                            ^
Peek

PeekToken(): DoGetToken():
        (WINAPI_PARTITION_DESKTOP|WINAPI_PARTITION_APP) & WINAPI_PARTITION_DESKTOP) == WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                                     ;                                                            ^
Save                                                                                                            ^
Undo                                                                                                            ^
Peek                                                                                                            _______

PeekToken(): End: Reset Token from Save:
Note that the m_Token string is untouched, it still
 contain semi-colon which we started with:
        (WINAPI_PARTITION_DESKTOP|WINAPI_PARTITION_APP) & WINAPI_PARTITION_DESKTOP) == WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                                     ;                                                     ^
Undo                                                                                                            ^
Peek                                                                                                            _______^

Now user runs UnGetToken(): m_PeekAvailable becomes true:
        (WINAPI_PARTITION_DESKTOP|WINAPI_PARTITION_APP) & WINAPI_PARTITION_DESKTOP) == WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                                                                                           ^
Undo                                                                                                            ^
Peek                                                      ;                                                     ^

GetToken() again: Take from peek:
        (WINAPI_PARTITION_DESKTOP|WINAPI_PARTITION_APP) & WINAPI_PARTITION_DESKTOP) == WINAPI_PARTITION_DESKTOP)typedef RGBQUAD LPRGBQUAD;
Token                                                     ;                                                     ^
Undo                                                                                                            ^
Peek

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Several improvements to Code Completion plugin
« Reply #104 on: June 26, 2014, 02:43:09 am »
Hi, Huki, thanks for the detailed explanation. I understand the issue. Indeed, if the m_Buffer is modified when macro replacement happens(it can either happend in DoGetToken() call or SkipUnwanted() call), both the "old token index"(m_SavedTokenIndex) and "undo token index"(m_UndoTokenIndex) will becomes invalid. A simple example is that if we have a macro replacement rule:

Code
AAA->BBBBBBBBBBBBBBBBBBBBBBBBBBBBB

After the replacement, the BBBBBBBBBBBBBBBBBBBBBBBBBBBBB can overwrite the m_Buffer[m_SavedTokenIndex], and m_Buffer[m_UndoTokenIndex], so both m_SavedTokenIndex and m_UndoTokenIndex should be reset. Since m_Token is still valid, we can still safely call void Tokenizer::UngetToken() once. In-fact, UngetToken() won't move any Indexs if they are reset.

I will commit your patch soon, thanks.

EDIT committed in r9834
« Last Edit: June 26, 2014, 03:02:36 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.