Code::Blocks

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: Huki on September 06, 2013, 10:03:35 pm

Title: Several improvements to Code Completion plugin
Post by: Huki on September 06, 2013, 10:03:35 pm
Hello,
I've been using CB for a few months now and love it. :) As I started noticing some parsing bugs and areas of improvement in CC features, I decided to take a look at it and have so far managed to fix most of the things I had in mind. I've attached a patch file and commented all my changes, let me know if something is not clear.

To summarize the important changes:
1) Full file parsing (i.e., expand each #include file and recurse), no need to keep priority headers. Previously when the parser finds an #include token, the file was simply added to the end of the parsing queue, so although all the include files are getting parsed anyway, it is done in the wrong order. Now the #include file parsing is done immediately instead of getting queued. It also means there is no need for 3-phase parsing (priority headers, non-priority headers, sources) anymore, simply parsing the source files is enough.

2) The "Grey out inactive code" feature now uses CC plugin support. I had to study carefully the CB SDK vs Scintilla communication to get this done in a clean way. I'm happy with my current implementation but let me know if it needs improvement. Despite the limitations of CC plugin, this feature is working nicely for most practical purposes. In case a token is #defined and used within the same file, their order is checked too. This ensures header files (with header guards) do not get totally greyed out.

3) CC support for DirectX (and related typedefs) and full WinAPI (including macro and function calltips). Parses function pointers and variables defined with constructor (eg., wxString myString(_T("text")); ), recursive union inside union, etc. and several other fixes. Improved tooltip behavior and speed, for both CC and Debugger.

With my 2008 PC (Core 2 Duo 2.6 GHz) and all parsing options enabled (parse global / local headers, preprocessor, complex macros) it takes about 20sec to parse the full CodeBlocks project, not bad at all. :)
All changes are checked with rev 9271.


[attachment deleted by admin]
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 06, 2013, 11:02:57 pm
Can you provide separate patches for all different fixes, so they can be reviewed on a per fix basis?
Reviewing one massive patch is a night mare and minimizes the chance of a fix getting in.
Title: Re: Several improvements to Code Completion plugin
Post by: BlueHazzard on September 07, 2013, 12:23:19 am
also look at this thread: http://forums.codeblocks.org/index.php/topic,18114.45.html
i think alpha has made some changes, maybe you both can sync your patches (i don't have revised any of the patches, but i thought i will inform you that there is ongoing work on the CC plugin)

With my 2008 PC (Core 2 Duo 2.6 GHz) and all parsing options enabled (parse global / local headers, preprocessor, complex macros) it takes about 20sec to parse the full CodeBlocks project, not bad at all. :)
All changes are checked with rev 9271.
these sounds great!

greetings
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 07, 2013, 06:00:26 am
Ok, I'll try to split the patch file for each fix / improvement. Several fixes belong to the same file, but I'll see what I can do.

@BlueHazzard: Thanks for the link, I'm not sure if they are working on the current CC plugin or something new entirely (seems like the latter). Will check it in detail soon. :)
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 07, 2013, 11:31:46 am
Ok, I'll try to split the patch file for each fix / improvement. Several fixes belong to the same file, but I'll see what I can do.
You can do it, pretty easily with git and git gui.
You can use the repo at https://github.com/obfuscated/codeblocks_sf

@BlueHazzard: Thanks for the link, I'm not sure if they are working on the current CC plugin or something new entirely (seems like the latter). Will check it in detail soon. :)
In fact Alpha is extracting some API for writing CC plugins and he is modifying the current CC plugin to use the API.
I've looked at your changes and you're adding something similar, but you're doing it in a pretty unacceptable way (I'm talking about the wxdynamiclib stuff and all the modifications to wxscintilla). But I'm waiting for the separate patches to post some more thorough comments.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 07, 2013, 02:55:49 pm
@Huki, very good work, thanks.
I will apply the patch, and test it. It is better have a patch serials not a big one. :) As OBF said, git can splite a big patch and generate such patch serials.

About the parsing strategy, my concern is that recursive expanding include files in a cpp file will parse a lot of same header files for many times.
Title: Re: Several improvements to Code Completion plugin
Post by: Alpha on September 08, 2013, 12:20:27 am
I have not had a chance to look through your work (my priority is currently on CC API), but the features sound useful.  The pure parsing related parts, *should* have no conflicts with my modifications, I believe.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 08, 2013, 12:59:49 am
I decided to split the patch file manually with a text editor this time, I think it should work. I've attached the patches set below. I'll try out git hopefully tomorrow or next week, and sync my changes with the github repo.

Quote from: ollydbg
About the parsing strategy, my concern is that recursive expanding include files in a cpp file will parse a lot of same header files for many times.
Till now I had not checked this part in detail (i.e., whether already parsed files are parsed again and again, or not), but now I can confirm: already parsed headers actually get skipped. In fact it's not going to make a difference even if we change this behavior, because tokens are not managed per-file but project wide. In this case it only matters for us to ensure tokens are collected in the correct order. If they are already collected, we don't achieve anything by reparsing the file. So the accuracy is good enough to avoid the use of priority headers, while having almost no impact on performance (because we are still parsing the same number of files - in fact lesser as we don't have a separate headers parsing phase).
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 08, 2013, 01:25:57 am
Hm...

Can you post thorough descriptions for the problems you try to solve with every patch?
Steps to reproduce will be useful, too.

I'm particularly interested in the two debugger patches and the thread search one.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 08, 2013, 03:30:44 am
I'll quickly explain the patches you asked for, rest I'll try to do later (please let me know if there is anything else specifically).

debugger_tooltip:
- Updated the code for detecting whether the mouse pointer is over a selected text. The original code was just a simple approximation, this one does a proper test.
- Placed the tooltip below the mouse pointer, rather than directly on it. This makes it possible to select text (in the editor) even if the tooltip was triggered. In addition, the user must be able to move the mouse pointer into the tooltip (mainly to expand items in the tooltip), so we should not dismiss the tooltip as soon as the mouse is moved (give a safe area outside the tooltip where the mouse can move without accidentally dismissing the tooltip).
- Fixed both code completion tooltip and debugger tooltips appearing at the same time (it happens if debugger tooltip takes some time to compute and display). Several tests are done before displaying the debugger tooltip, one of which is supposed to avoid this bug from occuring. I've fixed the order in which those tests are done.
- Do not show the debugger tooltip when Ctrl key is pressed (i.e., if "Only show tooltip when Ctrl key is pressed" option is turned off - the default, we now do the opposite).

debugger_fixes:
- Debugger stalls when trying to step into unknown code (or even a return statement). See this thread (http://forums.codeblocks.org/index.php/topic,17114.0.html).
- Setting the working directory for debugger fails for me, tracked it down to wxSetWorkingDirectory() which fails if the path is converted to GDB format. Removing the ConvertToGDBDirectory(path); fixes it.

thread_search:
- Do not automatically prepend the tilde sign if found. Users can still select the text including the ~ if they wish to include it in the search text. I have seen several cases where the tilde is used as the bitwise NOT operator, so shouldn't always assume for C++ destructor. I've also done similar fixes in Code Completion plugin.
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 08, 2013, 11:26:29 am
- Updated the code for detecting whether the mouse pointer is over a selected text. The original code was just a simple approximation, this one does a proper test.
Can you provide the steps, where the old one failed?

- Fixed both code completion tooltip and debugger tooltips appearing at the same time (it happens if debugger tooltip takes some time to compute and display). Several tests are done before displaying the debugger tooltip, one of which is supposed to avoid this bug from occuring. I've fixed the order in which those tests are done.
This is more of a black magic, not science, but I'll see if it makes a difference.

- Do not show the debugger tooltip when Ctrl key is pressed (i.e., if "Only show tooltip when Ctrl key is pressed" option is turned off - the default, we now do the opposite).
I still don't understand...Steps to reproduce?

- Debugger stalls when trying to step into unknown code (or even a return statement). See this thread (http://forums.codeblocks.org/index.php/topic,17114.0.html).
I think this is fixed already. Are you able to reproduce the problem without this patch using latest trunk?

- Setting the working directory for debugger fails for me, tracked it down to wxSetWorkingDirectory() which fails if the path is converted to GDB format. Removing the ConvertToGDBDirectory(path); fixes it.
Hm, I'm not sure if this is not going to break something else...

thread_search:
- Do not automatically prepend the tilde sign if found. Users can still select the text including the ~ if they wish to include it in the search text. I have seen several cases where the tilde is used as the bitwise NOT operator, so shouldn't always assume for C++ destructor. I've also done similar fixes in Code Completion plugin.
It sounds like we need to move the cbDebuggerPlugin::GetEditorWordAtCaret to the EditorManager class, so all plugins can reuse it or in cbPlugin...
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 08, 2013, 06:06:46 pm
- Updated the code for detecting whether the mouse pointer is over a selected text. The original code was just a simple approximation, this one does a proper test.
Can you provide the steps, where the old one failed?
When a line is not fully selected, the old test would detect the unselected parts of the line as selected and display tooltip on it (we only want to display debugger tooltip on the selected text).

Quote
- Fixed both code completion tooltip and debugger tooltips appearing at the same time (it happens if debugger tooltip takes some time to compute and display). Several tests are done before displaying the debugger tooltip, one of which is supposed to avoid this bug from occuring. I've fixed the order in which those tests are done.
This is more of a black magic, not science, but I'll see if it makes a difference.
Hmm no, we need to make sure the "get rid of other calltips" is the first test to be done. If there are any tests done before it, and they already fail and return, the existing tooltips never gets canceled, hence the bug.

Quote
- Do not show the debugger tooltip when Ctrl key is pressed (i.e., if "Only show tooltip when Ctrl key is pressed" option is turned off - the default, we now do the opposite).
I still don't understand...Steps to reproduce?
This one not a bugfix, I've added a small feature: press and hold Ctrl key to temporarily disable the debugger tooltip.

Quote
- Debugger stalls when trying to step into unknown code (or even a return statement). See this thread (http://forums.codeblocks.org/index.php/topic,17114.0.html).
I think this is fixed already. Are you able to reproduce the problem without this patch using latest trunk?
When I updated to rev 9271 I noticed there was an attempt to fix this. I haven't tested if it works. I still found my modification relevant (I commented that part). RunQueue() is already called at the end of the procedure, so we'd better flag the m_ProgramIsStopped, and let RunQueue() reset it if required.

Quote
- Setting the working directory for debugger fails for me, tracked it down to wxSetWorkingDirectory() which fails if the path is converted to GDB format. Removing the ConvertToGDBDirectory(path); fixes it.
Hm, I'm not sure if this is not going to break something else...
I can confirm the path is only used with wxSetWorkingDirectory(), never sent to GDB. So at least for now it should be safe to remove ConvertToGDBDirectory();
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 08, 2013, 10:14:57 pm
- Updated the code for detecting whether the mouse pointer is over a selected text. The original code was just a simple approximation, this one does a proper test.
Can you provide the steps, where the old one failed?
When a line is not fully selected, the old test would detect the unselected parts of the line as selected and display tooltip on it (we only want to display debugger tooltip on the selected text).
Additional info: the old test simply takes a rect from selection start to selection end, so it gives incorrect results when selecting multiple lines. I've attached some screenshots below - the red rectangle shows the incorrectly detected selection block. It's something that bothered me when I was selecting text (to copy, etc) and noticed the tooltip appearing behavior was not consistent.


I also checked if any of my other patches were in need of explanation:
scintilla_fixes: In LexerCPP::Fold(), fixed folding glitch in case of hanging braces in a preprocessor block. Eg,
Code: [Select]
#ifdef _DEFINE
    if (condition1) {
#else
    if (condition2) {
#endif
        ...
    }

jump_tracker: More consistent behavior for Jump Tracker (cursor history navigation) feature (View -> Jump -> Jump Back / Frwd - I set Ctrl - Left / Right shortcuts for it as it's a very useful feature for me :) ).

incremental_search: Fixed search history flooding. Eg, when typing "codecomp" in the search box, it used to record in history:
Code: [Select]
c
co
cod
code
...

The rest of patches are CC related...
cc_fix_tokenizer: In Tokenizer::ReadToEOL(), which is used by HandleDefines(),
Code: [Select]
   //FIX(huki) if (ch <= _T(' ') && (p == buffer || *(p - 1) == ch))
    if (ch <= _T(' ') && p > buffer && *(p - 1) == ch)
i.e., do not remove the leading space. This is to differentiate defines containing brackets, from legitimate macros. Eg,
Code: Text
  1. #define MYMACRO() ...
Code: Text
  1. #define MYDEFINE (A + B)

The fix in Tokenizer::ReadParantheses() support '==' (previously it was taken as two assignments, i.e., ' = = '.
Rest should be commented already in the patch.
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 08, 2013, 10:48:10 pm
One note:
Scintilla fixes should go to the Scintilla project first.
Here we do use a modified Scintilla, but we prefer not to diverge too much from upstream Scintilla.
It eases the upgrade of the component a lot.

Also I think your wxDynamicLib modification to (wx)Scintilla are too much and we cannot accept them (this is my opinion and others should speak, too, especially Morten as he is the maintainer of this component in C::B).
You'll have to find another way to do it. Probably consulting with Scintilla devs for a way to do it or to ask them to add some API that will suit you.
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 08, 2013, 11:07:56 pm
Additional info: the old test simply takes a rect from selection start to selection end, so it gives incorrect results when selecting multiple lines. I've attached some screenshots below - the red rectangle shows the incorrectly detected selection block. It's something that bothered me when I was selecting text (to copy, etc) and noticed the tooltip appearing behavior was not consistent.
Understood and I was able to reproduce it. Also I was able to implement it in a simpler way:
Code: [Select]
            int startPos = stc->GetSelectionStart();
            int endPos = stc->GetSelectionEnd();
            int mousePos = stc->PositionFromPointClose(mousePosition->x, mousePosition->y);
            if (mousePos == wxSCI_INVALID_POSITION)
                return wxEmptyString;
            else if (startPos <= mousePos && mousePos <= endPos)
                return selected_text;
            else
                return wxEmptyString;
Can you try if this snippet fulfils all your requirements?

BTW: Thank you for you contribution, it is appreciated.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 09, 2013, 11:09:54 pm
Additional info: the old test simply takes a rect from selection start to selection end, so it gives incorrect results when selecting multiple lines. I've attached some screenshots below - the red rectangle shows the incorrectly detected selection block. It's something that bothered me when I was selecting text (to copy, etc) and noticed the tooltip appearing behavior was not consistent.
Understood and I was able to reproduce it. Also I was able to implement it in a simpler way:

Can you try if this snippet fulfils all your requirements?
Yes, your code does the job nicely.

Quote
One note:
Scintilla fixes should go to the Scintilla project first.
Ok, I agree.

Quote
Also I think your wxDynamicLib modification to (wx)Scintilla are too much and we cannot accept them (this is my opinion and others should speak, too, especially Morten as he is the maintainer of this component in C::B).
You'll have to find another way to do it. Probably consulting with Scintilla devs for a way to do it or to ask them to add some API that will suit you.
Avoiding wxDynamicLib in Scintilla should be easy, I'll update that part soon.
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 10, 2013, 10:31:12 am
Yes, your code does the job nicely.
In svn...
Title: Re: Several improvements to Code Completion plugin
Post by: Alpha on September 10, 2013, 03:48:37 pm
Yes, your code does the job nicely.
In svn...
The following is more succinct (but perhaps less readable?), and has the same execution.
Code: [Select]
Index: src/sdk/cbplugin.cpp
===================================================================
--- src/sdk/cbplugin.cpp (revision 9299)
+++ src/sdk/cbplugin.cpp (working copy)
@@ -207,12 +207,10 @@
             int startPos = stc->GetSelectionStart();
             int endPos = stc->GetSelectionEnd();
             int mousePos = stc->PositionFromPointClose(mousePosition->x, mousePosition->y);
-            if (mousePos == wxSCI_INVALID_POSITION)
+            if (startPos > mousePos || endPos < mousePos)
                 return wxEmptyString;
-            else if (startPos <= mousePos && mousePos <= endPos)
+            else
                 return selected_text;
-            else
-                return wxEmptyString;
         }
         else
             return selected_text;
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 10, 2013, 04:14:31 pm
Don't like... I prefer the check for wxSCI_INVALID_POSITION
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 11, 2013, 09:09:28 pm
I have updated the scintilla grey out patch, let me know what you think of this one.
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 11, 2013, 10:31:17 pm
1. Using PluginManager and GetDynamicLibrary is still bad. Please checkout the work done here: https://github.com/alpha0010/codeblocks_sf/tree/cc_interface
   This branch tries to implement a CCManager and similar stuff. There is a topic in the forum and I think Ollydbg posted a link to it in a previous comment.
2. You still modify the lexer for C++, why don't you just find the defines for the file after parsing the includes and pass them to Scintilla, I think it is possible.
3. Using char* to store strings is highly discouraged.
4. I think this will be one of the harder to apply patches, because it does weird things in many subsystems. Probably you should concentrate on the simpler things getting in and then this one.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 13, 2013, 04:32:02 pm
I got an error when apply one patch:
Quote
$ patch <../patches/CodeBlocks_patch_huki_130906/cc_enum_values.patch -p0
patching file `src/plugins/codecompletion/parser/expression.cpp'
patching file `src/plugins/codecompletion/parser/expression.h'
patch: **** malformed patch at line 36: Index: src/plugins/codecompletion/parser
/parserthread.cpp

EDIT: problem solved, I found that an empty line is missing, that is mostly caused by manually editing/spliting the patch file
Code: [Select]
@@ -88,7 +88,7 @@
 
     PostfixVector m_PostfixExpression;
     InfixVector   m_InfixExpression;
-    bool          m_Result;
+    long          m_Result; //MOD(huki), return type was (bool)
     bool          m_Status;
 };
 
Index: src/plugins/codecompletion/parser/parserthread.cpp
Note that there should be an empty line before the line "Index:.....".



About other patches:
cc_build_target_handling.patch
If a build target is changed, reparse the whole project, I think it is not necessory. E.g. If we change the build target in Codeblocks.cbp, the whole project get reparsed? That's not necessory I think. If the user do want to a reparse, he can right click on the context menu of the project manager, and select "reparse this project" sub menu.

cc_includes_parsing.patch
I think this is OK, priority header parsing is not needed here. The only issue is that some standalone header files included in a project, but those include files were not used/included in any cpp files, so they will be skipped by the parser.

Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on September 13, 2013, 04:37:24 pm
About other patches:
cc_build_target_handling.patch
If a build target is changed, reparse the whole project, I think it is not necessory. E.g. If we change the build target in Codeblocks.cbp, the whole project get reparsed? That's not necessory I think. If the user do want to a reparse, he can right click on the context menu of the project manager, and select "reparse this project" sub menu.
ollydbg: The user should never have to click reparse, never, everything should be automatic. We even should remove it.

Also keep in mind that very few people do project management as we do. I'm sure most people use multiple projects+debug/release targets instead of the C::B's approach.
So if this patch helps for such scenarios we should consider it.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 13, 2013, 05:20:25 pm
About cc_enum_values.patch
I think it is OK. I see you want to use the expression solver to calculate the enum value, which means if you are going to parse the code:

Code: [Select]
enum A
{
x = 1+2;
}

Then, you will get the correct enum value of x. I think a simple test code is necessary, you can simply follow the file under: \src\plugins\codecompletion\testing, maybe the enumerator.cpp, thanks.

Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 13, 2013, 05:37:20 pm
Hm...

Can you post thorough descriptions for the problems you try to solve with every patch?
Steps to reproduce will be useful, too.
About the CC part: I'm interested on the patch: cc_fix_tokenizer.patch, cc_fix_ctor_dtor.patch, thanks. Maybe other cc patches, but I'm not viewing them yet. ;)
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 14, 2013, 07:51:31 am
1. Using PluginManager and GetDynamicLibrary is still bad. Please checkout the work done here: https://github.com/alpha0010/codeblocks_sf/tree/cc_interface
   This branch tries to implement a CCManager and similar stuff. There is a topic in the forum and I think Ollydbg posted a link to it in a previous comment.
My main concern was to safely pass the CC function pointer for Scintilla's use. Using PluginManager we ensure that each cbEditor can get hold of the CC library during its lifetime and prevents the plugin from being accidentally unloaded (eg, upon user uninstall). The good thing is that the CodeCompletion object along with the parsers and everything will get freed as the user expects, but as the library still remains loaded, it is safe for Scintilla to keep calling our eval function (which will simply report that CC isn't enabled). Once all the open cbEditors are closed the library will go away too.

I see about the CCManager work. It would definitely make it easier for the CB SDK to communicate with CC for features like these, still don't know about Scintilla. Perhaps working with PluginManager through CCManager would be cleaner rather than doing it directly from cbEditor like now.

Quote
2. You still modify the lexer for C++, why don't you just find the defines for the file after parsing the includes and pass them to Scintilla, I think it is possible.
3. Using char* to store strings is highly discouraged.
There is not only #ifdef style defines, we also need to handle defines with value, or defined to other defines, function-like macros. The Scintilla lexer's expression parser is also bare-bones... on the other hand CC's expression solver will give us consistent results. Letting Scintilla call a CC function to get the result seems like the best option before me.
What I could do is limit all Scintilla modifications to LexerCPP.cxx. Using Scintilla lexer's PrivateCall() support this should be possible. It will also eliminate the need for char*.

Quote
4. I think this will be one of the harder to apply patches, because it does weird things in many subsystems. Probably you should concentrate on the simpler things getting in and then this one.
I was going to suggest the same actually. I'll put this one on hold till we've dealt with the other patches.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 14, 2013, 07:58:20 am
Quote from: ollydbg
I got an error when apply one patch:
Oh, sorry about that. I took care to keep extra lines, must have missed this one.

About cc_enum_values.patch
I think it is OK. I see you want to use the expression solver to calculate the enum value, which means if you are going to parse the code:

Code: [Select]
enum A
{
x = 1+2;
}

Then, you will get the correct enum value of x. I think a simple test code is necessary, you can simply follow the file under: \src\plugins\codecompletion\testing, maybe the enumerator.cpp, thanks.
I'm sending a patch file for \src\plugins\codecompletion\testing\enumerator.cpp to show some supported cases.
Note that my current implementation doesn't check for correct parent, it just accepts any parent (I'm talking about enums defined inside a particular class or namespace). Maybe I'll try to finish this part soon.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 14, 2013, 08:05:20 am
Hm...

Can you post thorough descriptions for the problems you try to solve with every patch?
Steps to reproduce will be useful, too.
About the CC part: I'm interested on the patch: cc_fix_tokenizer.patch, cc_fix_ctor_dtor.patch, thanks. Maybe other cc patches, but I'm not viewing them yet. ;)
For cc_fix_tokenizer.patch I already explained in a previous post. I'm quoting it again for convenience:

cc_fix_tokenizer: In Tokenizer::ReadToEOL(), which is used by HandleDefines(),
Code: [Select]
   //FIX(huki) if (ch <= _T(' ') && (p == buffer || *(p - 1) == ch))
    if (ch <= _T(' ') && p > buffer && *(p - 1) == ch)
i.e., do not remove the leading space. This is to differentiate defines containing brackets, from legitimate macros. Eg,
Code: Text
  1. #define MYMACRO() ...
Code: Text
  1. #define MYDEFINE (A + B)

The fix in Tokenizer::ReadParantheses() support '==' (previously it was taken as two assignments, i.e., ' = = '.
Rest should be commented already in the patch.


I'll discuss the others later tonight.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 16, 2013, 06:09:39 pm
For cc_fix_tokenizer.patch I already explained in a previous post. I'm quoting it again for convenience:
Thanks.

Quote
cc_fix_tokenizer: In Tokenizer::ReadToEOL(), which is used by HandleDefines(),
Code: [Select]
    //FIX(huki) if (ch <= _T(' ') && (p == buffer || *(p - 1) == ch))
    if (ch <= _T(' ') && p > buffer && *(p - 1) == ch)
i.e., do not remove the leading space. This is to differentiate defines containing brackets, from legitimate macros. Eg,
Code: Text
  1. #define MYMACRO() ...
Code: Text
  1. #define MYDEFINE (A + B)
I understand your change of the ReadToEOL(), I see that the only usage/call of the ReadToEOL function is in HandleDefines(), so here we should read the first token after the "#define", after that, we should check if there is a "(" directly after the token, if yes, then this is a function like macro, if not, this is a variable like macro.
The parameter stripUnneeded means: remove comments and compression spaces(two or more spaces should become one space), I think you change is OK. The string returned from the ReadToEOL have two cases:
If the string is beginning of the space -> this is a variable like macro definition, otherwise, it is a function like macro definition.
I think I need to update the comments in either HandleDefines and ReadToEOL function now. :)

Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 17, 2013, 12:14:02 am
I have updated the cc_enum_value patch:
- In case of enum assignment to previous enum, it now looks under the correct parent instead of doing a global search.
- If the expression cannot be evaluated, leave it blank instead of resetting to zero (further enums will also be left blank till the next successful assignment).

In case you've already applied my previous patch, note that I have removed my changes to tokentree.cpp / tokentree.h (in my previous patch I had added a new TokenTree::TokenExists() function).

I'm trying to get each patch committed one by one, let me know if this one is ok.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 17, 2013, 03:17:42 am
...
I think I need to update the comments in either HandleDefines and ReadToEOL function now. :)
When reading the function ReadToEOL, I see it first allocate a wxChar buffer, and then Append to a wxString, firstly I concern this code compile under wx 2.9.x, because wx 2.9.x use native internal buffers for different OS (under Linux, it use UTF8, under Windows, it use wchar_t), luckily, It don't have such issue, the reason is below:

Quote
wxChar is defined to be
char when wxUSE_UNICODE==0
wchar_t when wxUSE_UNICODE==1 (the default).

Also, wxString have a member function:
Quote
wxString &  Append (const wchar_t *pwz, size_t nLen)
  Appends the wide string literal psz with max length nLen.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 17, 2013, 05:25:07 am
I think I need to update the comments in either HandleDefines and ReadToEOL function now. :)
Ok, I extract a patch dedicated to ReadToEOL with some of comments added, see below:
Code: [Select]
From 276d59ccf760ee85a16170db86efa9e362368a0a Mon Sep 17 00:00:00 2001
From: asmwarrior <[email protected]>
Date: Tue, 17 Sep 2013 11:10:15 +0800
Subject: [PATCH] handle macro handling correctly, distinguish between function
 like macro definition and variable like definition

---
 src/plugins/codecompletion/parser/tokenizer.cpp | 15 ++++++++++++---
 src/plugins/codecompletion/parser/tokenizer.h   |  3 +++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/plugins/codecompletion/parser/tokenizer.cpp b/src/plugins/codecompletion/parser/tokenizer.cpp
index 221d5ac..c0c5bbe 100644
--- a/src/plugins/codecompletion/parser/tokenizer.cpp
+++ b/src/plugins/codecompletion/parser/tokenizer.cpp
@@ -438,8 +438,10 @@ wxString Tokenizer::ReadToEOL(bool nestBraces, bool stripUnneeded)
         wxChar* p = buffer;
         wxString str;
 
+        // loop all the physical lines in reading macro definition
         for (;;)
         {
+            // this while statement end up in a physical EOL '\n'
             while (NotEOF() && CurrentChar() != _T('\n'))
             {
                 while (SkipComment())
@@ -449,7 +451,12 @@ wxString Tokenizer::ReadToEOL(bool nestBraces, bool stripUnneeded)
                 if (ch == _T('\n'))
                     break;
 
-                if (ch <= _T(' ') && (p == buffer || *(p - 1) == ch))
+                // if we see two spaces in the buffer, we should drop the second one. Note, if the
+                // first char is space, we should always save it to buffer, this is to distinguish
+                // a function/variable like macro definition, e.g.
+                // #define MYMACRO(A)  ...   -> function like macro definition
+                // #define MYMACRO (A)  ...  -> variable like macro definition, note a space before '('
+                if (ch <= _T(' ') && p > buffer && *(p - 1) == ch)
                 {
                     MoveToNextChar();
                     continue;
@@ -475,16 +482,18 @@ wxString Tokenizer::ReadToEOL(bool nestBraces, bool stripUnneeded)
                 MoveToNextChar();
             }
 
+            // check to see it is a logical EOL, some long macro definition contains a backslash-newline
             if (!IsBackslashBeforeEOL() || IsEOF())
-                break;
+                break; //break the outer for loop
             else
             {
+                //remove the backslash-newline and goto next physical line
                 while (p > buffer && *(--p) <= _T(' '))
                     ;
                 MoveToNextChar();
             }
         }
-
+        // remove the extra spaces in the end of buffer
         while (p > buffer && *(p - 1) <= _T(' '))
             --p;
 
diff --git a/src/plugins/codecompletion/parser/tokenizer.h b/src/plugins/codecompletion/parser/tokenizer.h
index 3999252..dbc8d1b 100644
--- a/src/plugins/codecompletion/parser/tokenizer.h
+++ b/src/plugins/codecompletion/parser/tokenizer.h
@@ -188,6 +188,9 @@ public:
 
     /** return the string from the current position to the end of current line, in most case, this
      * function is used in handling #define, use with care outside this class!
+     * @param nestBraces true if you still need to count the '{' and '}' levels
+     * @param stripUnneeded true if you are going to remove comments and compression spaces(two or
+     * more spaces should become one space)
      */
     wxString ReadToEOL(bool nestBraces = true, bool stripUnneeded = true);
 
--
1.8.4.msysgit.0


PS: git is vary good at split commit. :)

EDIT: Here is the test case
Parsing code:
Code: [Select]
#define FUNCTION_LIKE_DEFINE(A) (A+B)
#define VARIABLE_LIKE_DEFINE (A) (A+B)
Parsing log:
Code: [Select]
000001. --------------M-a-i-n--L-o-g--------------


000002. -----------I-n-t-e-r-i-m--L-o-g-----------
000003. InitTokenizer() : m_Filename='C:\DOCUME~1\zyh23\LOCALS~2\Temp\cc20.h', m_FileSize=85.
000004. Init() : m_Filename='C:\DOCUME~1\zyh23\LOCALS~2\Temp\cc20.h'
000005. C:\DOCUME~1\zyh23\LOCALS~2\Temp\cc20.h
000006. Parse() : Parsing 'C:\DOCUME~1\zyh23\LOCALS~2\Temp\cc20.h'
000007. DoParse() : Loop:m_Str='', token='#'
000008. wxString Tokenizer::ReadToEOL(bool, bool) : line=2, CurrentChar='(', PreviousChar='E', NextChar='A', nestBrace(0)
000009. ReadToEOL(): (END) We are now at line 2, CurrentChar='\n', PreviousChar='\r', NextChar='#'
000010. ReadToEOL(): (A) (A+B)
000011. DoAddToken() : Created token='FUNCTION_LIKE_DEFINE', file_idx=1, line=2, ticket=258
000012. GetTokenBaseType() : Searching within m_Str='(A+B)'
000013. GetTokenBaseType() : Compensated m_Str='(A+B)'
000014. GetTokenBaseType() : Found ''
000015. DoAddToken() : Prepending ''
000016. DoAddToken() : Added/updated token 'FUNCTION_LIKE_DEFINE' (0), kind 'preprocessor', type '(A+B)', actual ''. Parent is  (-1)
000017. DoParse() : Loop:m_Str='', token='#'
000018. wxString Tokenizer::ReadToEOL(bool, bool) : line=3, CurrentChar=' ', PreviousChar='E', NextChar='(', nestBrace(0)
000019. ReadToEOL(): (END) We are now at line 3, CurrentChar='\n', PreviousChar='\r', NextChar='\r'
000020. ReadToEOL():  (A) (A+B)
000021. DoAddToken() : Created token='VARIABLE_LIKE_DEFINE', file_idx=1, line=3, ticket=259
000022. GetTokenBaseType() : Searching within m_Str=' (A) (A+B)'
000023. GetTokenBaseType() : Compensated m_Str=' (A) (A+B)'
000024. GetTokenBaseType() : Found ''
000025. DoAddToken() : Prepending ''
000026. DoAddToken() : Added/updated token 'VARIABLE_LIKE_DEFINE' (1), kind 'preprocessor', type ' (A) (A+B)', actual ''. Parent is  (-1)
********************************************************

Looks good to me.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 17, 2013, 07:22:13 am
About this code snippet:
Code: [Select]
        // Here, we are in the comment body
        while (true)
        {
            if (cstyle) // C style comment
            {
                //FIX(huki), Moved this from below to avoid taking the same '*' for comment begin and end
                // eg, /*// ... */
                if (!MoveToNextChar())
                    break;

                SkipToChar('/');
                if (PreviousChar() == '*') // end of a C style comment
                {
                    MoveToNextChar();
                    break;
                }
            }
            else        // C++ style comment
            {
                TRACE(_T("SkipComment() : Need to call SkipToInlineCommentEnd() here at line = %u"), m_LineNumber);
                SkipToInlineCommentEnd();
                break;
            }
        }

When begin, we have the index:
Code: [Select]
/*// ... */
  ^
If you run MoveToNextChar() first, you are now deliberately skip one character, and you are here in:
Code: [Select]
/*// ... */
   ^
Although this code did work correctly, but I think we can have a better method.
Method1: SkipToChar('*'); and see whether there is a '/' after '*'
Method2: if (PreviousChar() == '*' && CheckWeReallyMovedAfterSkipToChar)

I think method1 may be better, what's your opinion.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 17, 2013, 03:19:56 pm
I do not understand what this code snippet trying to solve, can you explain?
Code: [Select]
wxString Tokenizer::PeekToken()
{
    if (!m_PeekAvailable)
    {
        m_PeekAvailable = true;

        unsigned int savedTokenIndex = m_TokenIndex;
        unsigned int savedLineNumber = m_LineNumber;
        unsigned int savedNestLevel  = m_NestLevel;

        if (SkipUnwanted())
            m_PeekToken = DoGetToken();
        else
            m_PeekToken.Clear();

        m_PeekTokenIndex             = m_TokenIndex;
        m_PeekLineNumber             = m_LineNumber;
        m_PeekNestLevel              = m_NestLevel;

        //FIX(huki), check whether m_TokenIndex has decreased which implies a ReplaceBufferForReparse() was done.
        // We can also check for change in m_IsReplaceParsing and m_RepeatReplaceCount before and after DoGetToken().
        if (m_IsReplaceParsing && savedTokenIndex > m_TokenIndex)
            savedTokenIndex = m_TokenIndex;

        m_TokenIndex                 = savedTokenIndex;
        m_LineNumber                 = savedLineNumber;
        m_NestLevel                  = savedNestLevel;
    }

    return m_PeekToken;
}

I have try to let the cctest project parsing a simple buffer:
Code: [Select]
/********************************/
/********************************/
/********************************/
/********************************/

_STD_BEGIN

int a;
int b;

_STD_END

With the replacement rule:
Code: [Select]
    Tokenizer::SetReplacementString(_T("_STD_BEGIN"),                       _T("namespace std {"));
    Tokenizer::SetReplacementString(_T("_STD_END"),                         _T("}"));

But I don't see the if(m_IsReplaceParsing && savedTokenIndex > m_TokenIndex) condition is true.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 17, 2013, 11:01:45 pm
Ok, I extract a patch dedicated to ReadToEOL with some of comments added, see below:
Code: [Select]
From 276d59ccf760ee85a16170db86efa9e362368a0a Mon Sep 17 00:00:00 2001
From: asmwarrior <[email protected]>
Date: Tue, 17 Sep 2013 11:10:15 +0800
Subject: [PATCH] handle macro handling correctly, distinguish between function
 like macro definition and variable like definition

---
 src/plugins/codecompletion/parser/tokenizer.cpp | 15 ++++++++++++---
 src/plugins/codecompletion/parser/tokenizer.h   |  3 +++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/src/plugins/codecompletion/parser/tokenizer.cpp b/src/plugins/codecompletion/parser/tokenizer.cpp
index 221d5ac..c0c5bbe 100644
--- a/src/plugins/codecompletion/parser/tokenizer.cpp
+++ b/src/plugins/codecompletion/parser/tokenizer.cpp
@@ -438,8 +438,10 @@ wxString Tokenizer::ReadToEOL(bool nestBraces, bool stripUnneeded)
         wxChar* p = buffer;
         wxString str;
 
+        // loop all the physical lines in reading macro definition
         for (;;)
         {
+            // this while statement end up in a physical EOL '\n'
             while (NotEOF() && CurrentChar() != _T('\n'))
             {
                 while (SkipComment())
@@ -449,7 +451,12 @@ wxString Tokenizer::ReadToEOL(bool nestBraces, bool stripUnneeded)
                 if (ch == _T('\n'))
                     break;
 
-                if (ch <= _T(' ') && (p == buffer || *(p - 1) == ch))
+                // if we see two spaces in the buffer, we should drop the second one. Note, if the
+                // first char is space, we should always save it to buffer, this is to distinguish
+                // a function/variable like macro definition, e.g.
+                // #define MYMACRO(A)  ...   -> function like macro definition
+                // #define MYMACRO (A)  ...  -> variable like macro definition, note a space before '('
+                if (ch <= _T(' ') && p > buffer && *(p - 1) == ch)
                 {
                     MoveToNextChar();
                     continue;
@@ -475,16 +482,18 @@ wxString Tokenizer::ReadToEOL(bool nestBraces, bool stripUnneeded)
                 MoveToNextChar();
             }
 
+            // check to see it is a logical EOL, some long macro definition contains a backslash-newline
             if (!IsBackslashBeforeEOL() || IsEOF())
-                break;
+                break; //break the outer for loop
             else
             {
+                //remove the backslash-newline and goto next physical line
                 while (p > buffer && *(--p) <= _T(' '))
                     ;
                 MoveToNextChar();
             }
         }
-
+        // remove the extra spaces in the end of buffer
         while (p > buffer && *(p - 1) <= _T(' '))
             --p;
 
diff --git a/src/plugins/codecompletion/parser/tokenizer.h b/src/plugins/codecompletion/parser/tokenizer.h
index 3999252..dbc8d1b 100644
--- a/src/plugins/codecompletion/parser/tokenizer.h
+++ b/src/plugins/codecompletion/parser/tokenizer.h
@@ -188,6 +188,9 @@ public:
 
     /** return the string from the current position to the end of current line, in most case, this
      * function is used in handling #define, use with care outside this class!
+     * @param nestBraces true if you still need to count the '{' and '}' levels
+     * @param stripUnneeded true if you are going to remove comments and compression spaces(two or
+     * more spaces should become one space)
      */
     wxString ReadToEOL(bool nestBraces = true, bool stripUnneeded = true);
 
--
1.8.4.msysgit.0

Yes, that looks good.

About your test case:
Code: [Select]
000026. DoAddToken() : Added/updated token 'VARIABLE_LIKE_DEFINE' (1), kind 'preprocessor', type ' (A) (A+B)', actual ''. Parent is  (-1)You can see here type ' (A) (A+B)'. Which means the leading space is kept for "type". This problem will be addressed when you apply my parserthread.cpp patches (cc_parser_general.patch file). Here is the relevant part:
Code: Text
  1. Index: src/plugins/codecompletion/parser/parserthread.cpp
  2. ===================================================================
  3. --- src/plugins/codecompletion/parser/parserthread.cpp  (revision 9271)
  4. +++ src/plugins/codecompletion/parser/parserthread.cpp  (working copy)
  5. @@ -1294,7 +1345,7 @@
  6.  
  7.      Token* newToken = 0;
  8.      wxString newname(name);
  9. -    m_Str.Trim();
  10. +    m_Str.Trim(true).Trim(false);
  11.      if (kind == tkDestructor)
  12.      {
  13.          // special class destructors case
  14.  

For this subject, I suggest committing your ReadToEOL() patch and this DoAddToken() patch above.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 17, 2013, 11:18:04 pm
When begin, we have the index:
Code: [Select]
/*// ... */
  ^
If you run MoveToNextChar() first, you are now deliberately skip one character, and you are here in:
Code: [Select]
/*// ... */
   ^
Although this code did work correctly, but I think we can have a better method.
Method1: SkipToChar('*'); and see whether there is a '/' after '*'
Method2: if (PreviousChar() == '*' && CheckWeReallyMovedAfterSkipToChar)

I think method1 may be better, what's your opinion.
I don't think we need to change the method. A comment block always starts and ends with two chars, so deliberately skipping one char is a safe "shortcut" IMO. In the extreme case we will have:
Begin:
Code: [Select]
/**/
  ^
After running MoveToNextChar() first:
Code: [Select]
/**/
   ^
So it's going to be ok.
But if we really need more readable code, I suggest Method1 too.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 17, 2013, 11:35:55 pm
I do not understand what this code snippet trying to solve, can you explain?
Code: [Select]
       //FIX(huki), check whether m_TokenIndex has decreased which implies a ReplaceBufferForReparse() was done.
        // We can also check for change in m_IsReplaceParsing and m_RepeatReplaceCount before and after DoGetToken().
        if (m_IsReplaceParsing && savedTokenIndex > m_TokenIndex)
            savedTokenIndex = m_TokenIndex;

I have try to let the cctest project parsing a simple buffer:
With the replacement rule:
But I don't see the if(m_IsReplaceParsing && savedTokenIndex > m_TokenIndex) condition is true.

But with your test buffer, the replacement doesn't happen within PeekToken(). So of course, my fix won't be reached.
If we look at how PeekToken() works, we do the following:
Code: [Select]
       // 1) First we save existing token index:
        unsigned int savedTokenIndex = m_TokenIndex;
        unsigned int savedLineNumber = m_LineNumber;
        unsigned int savedNestLevel  = m_NestLevel;

        // 2) Then we get new token. Within DoGetToken(), a ReplaceBufferForReparse() may occur,
        //    either to expand an actual macro, or because of user replacement token.
        //    This will rewind m_TokenIndex, so the savedTokenIndex above will become incorrect.
        if (SkipUnwanted())
            m_PeekToken = DoGetToken();
        else
            m_PeekToken.Clear();

        // 3) Restore saved index. Because of above mentioned issue, this could end up restoring m_TokenIndex to wrong value.
        //    This will cause bad problems if UngetToken() is called after this PeekToken().
        m_TokenIndex                 = savedTokenIndex;
        m_LineNumber                 = savedLineNumber;
        m_NestLevel                  = savedNestLevel;

That is a PeekToken() specific fix. In general, we also need to reset m_UndoTokenIndex after a ReplaceBufferForReparse(). This is also done in the cc_fix_tokenizer.patch.
Code: [Select]
Index: src/plugins/codecompletion/parser/tokenizer.cpp
===================================================================
--- src/plugins/codecompletion/parser/tokenizer.cpp (revision 9271)
+++ src/plugins/codecompletion/parser/tokenizer.cpp (working copy)
@@ -1817,6 +1865,7 @@
 
     // Fix token index
     m_TokenIndex -= bufferLen;
+    m_UndoTokenIndex = m_TokenIndex;    //ADD(huki)
 
     // Update the peek token
     if (m_PeekAvailable && updatePeekToken)
Title: Re: Several improvements to Code Completion plugin
Post by: 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?
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 18, 2013, 04:10:40 am
...
So it's going to be ok.
But if we really need more readable code, I suggest Method1 too.
Thanks for the comments on this, I have commit to SVN trunk now with "Method1". :)

...
For this subject, I suggest committing your ReadToEOL() patch and this DoAddToken() patch above.
This is also committed to SVN trunk now.

These are the commits I have done. (My time is UTC+8 based)
Quote
Revision: 9343
Author: ollydbg
Date: 2013-9-18 9:59:45
Message:
* CC: when reading the parentheses, set the correct Tokenizer mode(tsReadRawExpression), also skip comments after the first '(' (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp

Revision: 9342
Author: ollydbg
Date: 2013-9-18 9:59:07
Message:
* CC: only skip after single equal sign, not double equals sign (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp

Revision: 9341
Author: ollydbg
Date: 2013-9-18 9:58:29
Message:
* CC: correctly find the end of a c style comment (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp

Revision: 9340
Author: ollydbg
Date: 2013-9-18 9:57:51
Message:
* CC: add missing breaks in switch case statements (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp

Revision: 9339
Author: ollydbg
Date: 2013-9-18 9:57:11
Message:
* CC: handle macro definition correctly, distinguish between function like macro definition and variable like definition (thanks Huki)
-------------------------------
M(T ) : /trunk/src/plugins/codecompletion/parser/parserthread.cpp

M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.cpp

M(T ) : /trunk/src/plugins/codecompletion/parser/tokenizer.h


Title: Re: Several improvements to Code Completion plugin
Post by: 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: [Select]
       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;

And here is the patch:
Code: Text
  1. Index: src/plugins/codecompletion/parser/tokenizer.cpp
  2. ===================================================================
  3. --- src/plugins/codecompletion/parser/tokenizer.cpp     (revision 9271)
  4. +++ src/plugins/codecompletion/parser/tokenizer.cpp     (working copy)
  5. @@ -713,7 +718,6 @@
  6.                  if (*(p - 1) <= _T(' '))
  7.                  {
  8.                      *p = _T('=');
  9. -                    *++p = _T(' ');
  10.                      ++p;
  11.                  }
  12.                  else
  13.  
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 19, 2013, 12:53:20 am
@huki: Can I ask you to provide a patch which adds you to the "help -> about -> thanks to" dialog?
Sure, here you go:

Code: [Select]
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"
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 20, 2013, 02:50:04 pm
@ollydbg: Thanks! I see one fix related to '==' sign hasn't made it through, in ReadParantheses(). See the code below with my comment.
Code: [Select]
       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;

And here is the patch:
Code: Text
  1. Index: src/plugins/codecompletion/parser/tokenizer.cpp
  2. ===================================================================
  3. --- src/plugins/codecompletion/parser/tokenizer.cpp     (revision 9271)
  4. +++ src/plugins/codecompletion/parser/tokenizer.cpp     (working copy)
  5. @@ -713,7 +718,6 @@
  6.                  if (*(p - 1) <= _T(' '))
  7.                  {
  8.                      *p = _T('=');
  9. -                    *++p = _T(' ');
  10.                      ++p;
  11.                  }
  12.                  else
  13.  

FYI: Morten has applied this patch (rev9351), thanks.
Title: Re: Several improvements to Code Completion plugin
Post by: 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.

As an example, we have this code:
Code: [Select]
#define MYMACRO X=0 /*Alternatively, user might have a replacement rule for MYMACRO*/
int X;
MYMACRO;
X = 1;

^ 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: [Select]
           int X;MYMACRO;X = 1;
Token:           _^
 Undo:           ^
Step 1: Update undo token and get next token.
Code: [Select]
           int X;MYMACRO;X = 1;
Token:            _______^
 Undo:            ^
Step 2: ReplaceBufferForReparse(): Replace the macro and reset token index.
BUG HERE: Undo token is still pointing to bad location!
Code: [Select]
           int X;MYMAX=0;X = 1;
Token:                ^
 Undo:            ^
FIX: Move the undo index to correct location.
Code: [Select]
           int X;MYMAX=0;X = 1;
Token:                ^
 Undo:                ^
Step 3: Get the next token now after replace.
Code: [Select]
           int X;MYMAX=0;X = 1;
Token:                _^
 Undo:                ^


CASE II: when ReplaceBufferForReparse() is called in PeekToken(), we have an additional problem.
PeekToken() Begin:
Step 1: Save the token index.
Code: [Select]
           int X;MYMACRO;X = 1;
Token:           _^
 Save:            ^

Step 2: DoGetToken():
 - Get next token.
Code: [Select]
           int X;MYMACRO;X = 1;
Token:            _______^
 Save:            ^
- Replace the macro and reset token index.
Code: [Select]
           int X;MYMAX=0;X = 1;
Token:                ^
 Save:            ^
- Get the next token now after replace.
Code: [Select]
           int X;MYMAX=0;X = 1;
Token:                _^
 Save:            ^

Step 3: Assign Token to Peek, then restore Token from Save.
BUG HERE: Save is still pointing to bad location!
Code: [Select]
           int X;MYMAX=0;X = 1;
 Peek:                _^
Token:           _^
FIX: Reset the token index to correct location.
Code: [Select]
           int X;MYMAX=0;X = 1;
 Peek:                _^
Token:           _    ^


Here is an updated patch with fixes for both cases.
Code: Text
  1. Index: src/plugins/codecompletion/parser/tokenizer.cpp
  2. ===================================================================
  3. --- src/plugins/codecompletion/parser/tokenizer.cpp     (revision 9271)
  4. +++ src/plugins/codecompletion/parser/tokenizer.cpp     (working copy)
  5. @@ -1135,6 +1146,8 @@
  6.          unsigned int savedLineNumber = m_LineNumber;
  7.          unsigned int savedNestLevel  = m_NestLevel;
  8.  
  9. +        int savedReplaceCount = m_IsReplaceParsing ? m_RepeatReplaceCount : -1;
  10. +
  11.          if (SkipUnwanted())
  12.              m_PeekToken = DoGetToken();
  13.          else
  14. @@ -1144,9 +1157,20 @@
  15.          m_PeekLineNumber             = m_LineNumber;
  16.          m_PeekNestLevel              = m_NestLevel;
  17.  
  18. -        m_TokenIndex                 = savedTokenIndex;
  19. -        m_LineNumber                 = savedLineNumber;
  20. -        m_NestLevel                  = savedNestLevel;
  21. +        // Check whether a ReplaceBufferForReparse() was done in DoGetToken().
  22. +        // We assume m_Undo... have already been reset in ReplaceBufferForReparse().
  23. +        if (m_IsReplaceParsing && savedReplaceCount != (int)m_RepeatReplaceCount)
  24. +        {
  25. +            m_TokenIndex             = m_UndoTokenIndex;
  26. +            m_LineNumber             = m_UndoLineNumber;
  27. +            m_NestLevel              = m_UndoNestLevel;
  28. +        }
  29. +        else
  30. +        {
  31. +            m_TokenIndex             = savedTokenIndex;
  32. +            m_LineNumber             = savedLineNumber;
  33. +            m_NestLevel              = savedNestLevel;
  34. +        }
  35.      }
  36.  
  37.      return m_PeekToken;
  38. @@ -1818,6 +1871,11 @@
  39.      // Fix token index
  40.      m_TokenIndex -= bufferLen;
  41.  
  42. +    // Fix undo position
  43. +    m_UndoTokenIndex = m_TokenIndex;
  44. +    m_UndoLineNumber = m_LineNumber;
  45. +    m_UndoNestLevel = m_NestLevel;
  46. +
  47.      // Update the peek token
  48.      if (m_PeekAvailable && updatePeekToken)
  49.      {
  50.  
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 28, 2013, 05:21:14 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.
...
Hi, thanks for the explanation, your patch is OK from my review. :D

Besides that, I think an improved UngetToken() can be:
Code: [Select]
/* 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;
}
We only allow the user run UngetToken() once,  not twice, right?
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 29, 2013, 10:39:27 am
@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. :)
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on January 04, 2014, 10:38:49 am
Today, I have tested the patch: cc_includes_parsing.patch
But I found it cause some GUI hang problem, see my report question about running parser in thread (http://forums.codeblocks.org/index.php/topic,18742.msg128410.html#msg128410), I guess the reason is:

If you recursively parse the #include directive.

Code: [Select]
                if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)
                //this is a recursive call to Parse() function????
                result = thread->Parse();
                delete thread;

                if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
So, you hold the s_TokenTreeMutex for a very long time. Some GUI function may try to query information from Tokentree, thus they get hangs.
Title: Re: Several improvements to Code Completion plugin
Post by: Folco on January 04, 2014, 11:45:03 am
What is approximatively the size of the data that you lock ? If it is not too huge, would it be a solution to lock a mutex, then copy the data that you need, then unlock the mutex ? It could be really faster that what you are currently doing.

I have this code in one of my projects :
Code: [Select]
        SDL_mutexP (rsd->RenderingMutex);
        rgd = rsd->rgd;
        SDL_mutexV (rsd->RenderingMutex);
This code is run in a rendering thread, which grabs only the data that it needs to draw, then unlock data for the main thread to continue. Perhaps something like that would remove some CC hangs and wouldn't freeze the GUI anymore.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on January 04, 2014, 01:44:48 pm
Hi, thanks Folco for the help.
Quote
What is approximatively the size of the data that you lock ?
The tokentree, which holds all the tokens(symbols), you can regard it as symbol database.
When we parse a file, you mainly do syntax analysis and add symbols one by one.

In the current implementation, we lock the tokentree when starting a single file, after a file is parsed, the locker is released. Something like this:

Code: [Select]
while (file queue is not empty)
{
    get one file from the queue
    lock the tokentree
    while (parsing file not finished)
    {
        syntax analysis
        add symbol to tokentree
        if (meet a #include directive)
        {
            add file to the queue(for later parsing)
        }
    }
    release the locker of tokentree
    remove the file from the queue
}

So, I think it is not good idea to put the locker around "add symbol to tokentree".

Code: [Select]
while (file queue is not empty)
{
    get one file from the queue
    while (parsing file not finished)
    {
        syntax analysis
        lock the tokentree
        add symbol to tokentree
        release the locker of tokentree
        if (meet a #include directive)
        {
            add file to the queue(for later parsing)
        }
    }
    remove the file from the queue
}
Because lock and release will be called too frequently.


With Huki's patch, the code becomes:

Code: [Select]
while (file queue is not empty)
{
    get one file from the queue
    lock the tokentree
    while (parsing file not finished)
    {
        syntax analysis
        add symbol to tokentree
        if (meet a #include directive)
        {
            recursive parse on the included source file
        }
    }
    release the locker of tokentree
    remove the file from the queue
}

So, you can get the correct token, as the #include directive is correctly expanded(it does the same thing as C preprocessor do), the time of the locker becomes longer.

I have no idea what is the correct idea, I think the GUI should not access to the tokentree when the parsing is not finished.

Title: Re: Several improvements to Code Completion plugin
Post by: Folco on January 04, 2014, 02:14:31 pm
And something like that :
Code: [Select]
lock the tokentree
copy the token tree
release the tokentree

while (file queue not empty)
{
    analysis / add symbol / fill queue with include directives, modifying copied_tokentree

    lock the tokentree, update tokentree with copied_tokentree, then release it
}

Note that the token tree should contain a var saying "I am (not) in a writable state/copiable state". I don't know what use it.

This should have 2 consequences :
- the token tree is locked during short peroid of time, without risk of infinite loop (recursive inclusion or other funky trick)
- nobody has to wait for the parser to finish its task. Even if a parser would crash, the GUI would keep responsive.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on January 04, 2014, 02:42:23 pm
And something like that :
Code: [Select]
lock the tokentree
copy the token tree
release the tokentree

while (file queue not empty)
{
    analysis / add symbol / fill queue with include directives, modifying copied_tokentree

    lock the tokentree, update tokentree with copied_tokentree, then release it
}

What does the above pseudo code used for? For parser? the copied tokentree are used for GUI?
Note A Tokentree is a very big object, so copy tokentree takes a lot of time. So, it is a shared object, can either write or read at a time.

I think if the parser is running, I mean the tokentree should not be accessed by GUI thread, the GUI should show some words like: parsing is still running. Or delay the operation until tokentree stabilized (parsing finished).



Title: Re: Several improvements to Code Completion plugin
Post by: Folco on January 04, 2014, 03:04:47 pm
And something like that :
Code: [Select]
lock the tokentree
copy the token tree
release the tokentree

while (file queue not empty)
{
    analysis / add symbol / fill queue with include directives, modifying copied_tokentree

    lock the tokentree, update tokentree with copied_tokentree, then release it
}

What does the above pseudo code used for? For parser?
Yes. First the parser create a copy, then it modifies its copy when parsing files, and once a file is parsed, it can update the token tree used by the GUI or other parts of CB.
Quote
the copied tokentree are used for GUI?
No. It uses the TokenTree which is updated after each file by the parser thread.

Quote
Note A Tokentree is a very big object, so copy tokentree takes a lot of time. So, it is a shared object, can either write or read at a time.
Ah. That is why my very first question was about the size of the token tree. Do you have any idea of the size in KB/MB ?

Of course, my idea would not be usable it the token tree was too big... The aim of what I propose is to avoid long periods of locking for the Token Tree.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on January 04, 2014, 03:09:22 pm
Note A Tokentree is a very big object, so copy tokentree takes a lot of time. So, it is a shared object, can either write or read at a time.
Ah. That is why my very first question was about the size of the token tree. Do you have any idea of the size in KB/MB ?

Of course, my idea would not be usable it the token tree was too big... The aim of what I propose is to avoid long periods of locking for the Token Tree.
Thanks for all your reply.
A tokentree is very big, for example for a large project (Codeblocks.cbp), its size if about 100MByte or 200MByte or even bigger. ;D
Title: Re: Several improvements to Code Completion plugin
Post by: Folco on January 04, 2014, 03:29:48 pm
Erf :D Too large to perform a complete copy.
Nevertheless it could still be possible with a differential copy. I don't know if it would be a good solution, I never worked on such big data.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on January 04, 2014, 04:09:20 pm
Erf :D Too large to perform a complete copy.
Nevertheless it could still be possible with a differential copy. I don't know if it would be a good solution, I never worked on such big data.
What do you mean by "differential copy"?
Partial copy? I think partial copy of the tokentree is also not possible, because symbols in Tokentree are referenced each other.
Title: Re: Several improvements to Code Completion plugin
Post by: Folco on January 04, 2014, 04:29:17 pm
Sorry, my english is very bad. I think that : "push to TokenTree the parts of the modified TokenTree which have been modified". But this seems hard if symbols refers to other ones.
Title: Re: Several improvements to Code Completion plugin
Post by: dekar on January 20, 2014, 01:18:14 pm
Is there some technique to make  Code Completion interpret defines?
for example:
Code: [Select]
((Some_struct*) someAddr)-> completing correctly. But
Code: [Select]
#define AVAR ((Some_struct*) someAddr)
AVAR->
  does not wokr.
It is rely annoying with embedded programming.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on January 28, 2014, 10:28:03 am
Today, I have tested the patch: cc_includes_parsing.patch
But I found it cause some GUI hang problem, see my report question about running parser in thread (http://forums.codeblocks.org/index.php/topic,18742.msg128410.html#msg128410), I guess the reason is:

If you recursively parse the #include directive.

Code: [Select]
                if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)
                //this is a recursive call to Parse() function????
                result = thread->Parse();
                delete thread;

                if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
So, you hold the s_TokenTreeMutex for a very long time. Some GUI function may try to query information from Tokentree, thus they get hangs.
I have a improvement patch to handle this (see attachment). The parser only parses cpp files in the project, recursive call the parse function when it meet the #include directive, the result is quite good.

The only issue about this is that if the project contains some header files(like a header only library project) which does not used by any cpp file, they will be left. I don't think this is a very big issue, because a user project will include those headers, then it still get parsed correctly.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on January 28, 2014, 10:33:27 am
Is there some technique to make  Code Completion interpret defines?
for example:
Code: [Select]
((Some_struct*) someAddr)-> completing correctly. But
Code: [Select]
#define AVAR ((Some_struct*) someAddr)
AVAR->
  does not wokr.
It is rely annoying with embedded programming.
I think currently it is not, so it should be a feature request.
We need to do Macro substation before we breakup the statement string before the caret..... Patches are welcome. ;)
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on February 02, 2014, 04:10:04 pm
Hi, been busy for a while...

Today, I have tested the patch: cc_includes_parsing.patch
But I found it cause some GUI hang problem, see my report question about running parser in thread (http://forums.codeblocks.org/index.php/topic,18742.msg128410.html#msg128410), I guess the reason is:

If you recursively parse the #include directive.

Code: [Select]
               if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)
                //this is a recursive call to Parse() function????
                result = thread->Parse();
                delete thread;

                if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
So, you hold the s_TokenTreeMutex for a very long time. Some GUI function may try to query information from Tokentree, thus they get hangs.
I have a improvement patch to handle this (see attachment). The parser only parses cpp files in the project, recursive call the parse function when it meet the #include directive, the result is quite good.

Yes, my cc_includes_parsing patch did cause the UI to hang-up a bit because of the recursive parsing. I just tried your patch which frees up the mutex for a while before recursing, and the result is indeed quite good. Thanks!
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on February 02, 2014, 04:20:34 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.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on February 08, 2014, 10:23:45 am
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.

Thanks, patches won't be forgotten !  :) But time is limited.
Applied in my local repo and works quite well. (when testing, I also found an bug introduced in rev 9601).

One question remains, what does this means:
Code: [Select]
           //FIX: Moved above.
            /*if (peek==ParserConsts::colon)
            {
                // bit specifier (eg, xxx:1)
                // -> walk to , or }
                SkipToOneOfChars(ParserConsts::commaclbrace);
            }*/
You are going to comment out this?
From my point of view, it should be removed, right? Because it was already handled before (in a if condition)
Code: [Select]
       if (peek==ParserConsts::colon)
        {
            peek = SkipToOneOfChars(ParserConsts::equals + ParserConsts::commaclbrace);
        }


About the include file expansion patch:
Hi, been busy for a while...
Today, I have tested the patch: cc_includes_parsing.patch
But I found it cause some GUI hang problem, see my report question about running parser in thread (http://forums.codeblocks.org/index.php/topic,18742.msg128410.html#msg128410), I guess the reason is:

If you recursively parse the #include directive.

Code: [Select]
               if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)
                //this is a recursive call to Parse() function????
                result = thread->Parse();
                delete thread;

                if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
So, you hold the s_TokenTreeMutex for a very long time. Some GUI function may try to query information from Tokentree, thus they get hangs.
I have a improvement patch to handle this (see attachment). The parser only parses cpp files in the project, recursive call the parse function when it meet the #include directive, the result is quite good.

Yes, my cc_includes_parsing patch did cause the UI to hang-up a bit because of the recursive parsing. I just tried your patch which frees up the mutex for a while before recursing, and the result is indeed quite good. Thanks!

I'm not sure the change of my improvement patch was good enough, I just add a small sleep function, any comments about such hack??


Thanks again for your contribution.




Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on February 09, 2014, 04:03:03 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.

Thanks, patches won't be forgotten !  :) But time is limited.
Applied in my local repo and works quite well. (when testing, I also found an bug introduced in rev 9601).

One question remains, what does this means:
Code: [Select]
           //FIX: Moved above.
            /*if (peek==ParserConsts::colon)
            {
                // bit specifier (eg, xxx:1)
                // -> walk to , or }
                SkipToOneOfChars(ParserConsts::commaclbrace);
            }*/
You are going to comment out this?
From my point of view, it should be removed, right? Because it was already handled before (in a if condition)
Code: [Select]
       if (peek==ParserConsts::colon)
        {
            peek = SkipToOneOfChars(ParserConsts::equals + ParserConsts::commaclbrace);
        }
I think the answer is YES, so I commit the patch to trunk now rev 9642, thanks!
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on February 12, 2014, 03:40:34 pm
@Huki, some regression was reported, see Re: The 08 February 2014 build (9639) is out. (http://forums.codeblocks.org/index.php/topic,18916.msg129658.html#msg129658), so I revert rev 9369, I'm not sure which cause this hang issue, because our CC's code is a little mass, sorry about that. If you have time, you can help to clean up the code, thanks!
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on February 15, 2014, 03:50:49 am
About the include file expansion patch:
Hi, been busy for a while...
Today, I have tested the patch: cc_includes_parsing.patch
But I found it cause some GUI hang problem, see my report question about running parser in thread (http://forums.codeblocks.org/index.php/topic,18742.msg128410.html#msg128410), I guess the reason is:

If you recursively parse the #include directive.

Code: [Select]
               if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)
                //this is a recursive call to Parse() function????
                result = thread->Parse();
                delete thread;

                if (!locked)
                    CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
So, you hold the s_TokenTreeMutex for a very long time. Some GUI function may try to query information from Tokentree, thus they get hangs.
I have a improvement patch to handle this (see attachment). The parser only parses cpp files in the project, recursive call the parse function when it meet the #include directive, the result is quite good.

Yes, my cc_includes_parsing patch did cause the UI to hang-up a bit because of the recursive parsing. I just tried your patch which frees up the mutex for a while before recursing, and the result is indeed quite good. Thanks!

I'm not sure the change of my improvement patch was good enough, I just add a small sleep function, any comments about such hack??

It's actually working very well - I have no more hangs now during batch parsing. I don't have a problem with the use of sleep function, it achieves the purpose, i.e., release the mutex before each include file, (very similar to the old behavior before the includes patch when the files were simply added to the end of the queue). The proper solution would probably be to ensure that the tokentree mutex is not accessed by the GUI while parsing, but this is not very easy.. lot of things the user does - mouse hover over a symbol, clicking into a function block, etc., will trigger the GUI to try access the mutex.. those are the times when I noticed the hangs actually.
In short, I'm happy with the current solution. :)
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on February 16, 2014, 03:00:30 pm
...
...
It's actually working very well - I have no more hangs now during batch parsing. I don't have a problem with the use of sleep function, it achieves the purpose, i.e., release the mutex before each include file, (very similar to the old behavior before the includes patch when the files were simply added to the end of the queue).
Thanks, yes, it works well (on my winXP system), so if no objections, I will commit this change.

Quote
The proper solution would probably be to ensure that the tokentree mutex is not accessed by the GUI while parsing, but this is not very easy.. lot of things the user does - mouse hover over a symbol, clicking into a function block, etc., will trigger the GUI to try access the mutex.. those are the times when I noticed the hangs actually.
The old behavior, when the parser is working, then gui access to tokentree was forbidden until parser finishes batch parsing, user will see some tips like "Parser is running, so no tip available". And later Loaden (one of C::B dev, but he was not active for years now) added many lockers to let the tree access from multiply thread.

So, do you think we need to change back to the old behavior?  I hate those lockers. :)

Quote
In short, I'm happy with the current solution. :)
Sure, me to, we can get more accurate tokens.

Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on February 16, 2014, 03:07:47 pm
...
...
It's actually working very well - I have no more hangs now during batch parsing. I don't have a problem with the use of sleep function, it achieves the purpose, i.e., release the mutex before each include file, (very similar to the old behavior before the includes patch when the files were simply added to the end of the queue).
Thanks, yes, it works well (on my winXP system), so if no objections, I will commit this change.


BTW: any one can test the patch(in this reply (http://forums.codeblocks.org/index.php/topic,18315.msg129281.html#msg129281)) on Linux? Thanks.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on February 25, 2014, 07:44:50 am
...
...
It's actually working very well - I have no more hangs now during batch parsing. I don't have a problem with the use of sleep function, it achieves the purpose, i.e., release the mutex before each include file, (very similar to the old behavior before the includes patch when the files were simply added to the end of the queue).
Thanks, yes, it works well (on my winXP system), so if no objections, I will commit this change.
I'm ok with committing this patch, can confirm it works fine on Win 7 / 8.

Quote
The proper solution would probably be to ensure that the tokentree mutex is not accessed by the GUI while parsing, but this is not very easy.. lot of things the user does - mouse hover over a symbol, clicking into a function block, etc., will trigger the GUI to try access the mutex.. those are the times when I noticed the hangs actually.
The old behavior, when the parser is working, then gui access to tokentree was forbidden until parser finishes batch parsing, user will see some tips like "Parser is running, so no tip available". And later Loaden (one of C::B dev, but he was not active for years now) added many lockers to let the tree access from multiply thread.

So, do you think we need to change back to the old behavior?  I hate those lockers. :)
We can try changing back to the old behavior, I don't see any reason to access the tokentree before parsing is finished (we don't display tooltips anyway). Is it this kind of check that we find in NativeParser::MarkItemsByAI()?
Code: [Select]
    if (!m_Parser->Done())
    {
        wxString msg(_("The Parser is still parsing files."));
        msg += m_Parser->NotDoneReason();
        CCLogger::Get()->DebugLog(msg);
        return 0;
    }

Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on February 25, 2014, 07:49:21 am
...
...
It's actually working very well - I have no more hangs now during batch parsing. I don't have a problem with the use of sleep function, it achieves the purpose, i.e., release the mutex before each include file, (very similar to the old behavior before the includes patch when the files were simply added to the end of the queue).
Thanks, yes, it works well (on my winXP system), so if no objections, I will commit this change.
I'm ok with committing this patch, can confirm it works fine on Win 7 / 8.
Thanks for the test.

Quote
Quote
The proper solution would probably be to ensure that the tokentree mutex is not accessed by the GUI while parsing, but this is not very easy.. lot of things the user does - mouse hover over a symbol, clicking into a function block, etc., will trigger the GUI to try access the mutex.. those are the times when I noticed the hangs actually.
The old behavior, when the parser is working, then gui access to tokentree was forbidden until parser finishes batch parsing, user will see some tips like "Parser is running, so no tip available". And later Loaden (one of C::B dev, but he was not active for years now) added many lockers to let the tree access from multiply thread.

So, do you think we need to change back to the old behavior?  I hate those lockers. :)
We can try changing back to the old behavior, I don't see any reason to access the tokentree before parsing is finished (we don't display tooltips anyway). Is it this kind of check that we find in NativeParser::MarkItemsByAI()?
Code: [Select]
    if (!m_Parser->Done())
    {
        wxString msg(_("The Parser is still parsing files."));
        msg += m_Parser->NotDoneReason();
        CCLogger::Get()->DebugLog(msg);
        return 0;
    }


Yes, m_Parser->Done() is the condition check to see whether Parser is currently working(parsing).
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on February 28, 2014, 05:52:49 am
I'm ok with committing this patch, can confirm it works fine on Win 7 / 8.
Thanks for the test.
In SVN trunk now(r9665), it also includes a lot of code clean up, thanks for contribution.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on March 01, 2014, 07:24:47 pm
I'm ok with committing this patch, can confirm it works fine on Win 7 / 8.
Thanks for the test.
In SVN trunk now(r9665), it also includes a lot of code clean up, thanks for contribution.
Thanks for the commit.. just one question about this change in Parser::OnAllThreadsDone()
Code: [Select]
Index: src/plugins/codecompletion/parser/parser.cpp
===================================================================
--- src/plugins/codecompletion/parser/parser.cpp (revision 9664)
+++ src/plugins/codecompletion/parser/parser.cpp (revision 9665)
@@ -931,9 +888,7 @@
 
     // Do next task
     if (   !m_PoolTask.empty()
-        || !m_BatchParseFiles.empty()
-        || !m_PriorityHeaders.empty()
-        || !m_PredefinedMacros.IsEmpty() )
+        || !m_BatchParseFiles.empty() )
     {
         TRACE(_T("Parser::OnAllThreadsDone(): Still some tasks left, starting m_BatchTimer."));
         m_BatchTimer.Start(ParserCommon::PARSER_BATCHPARSE_TIMER_RUN_IMMEDIATELY, wxTIMER_ONE_SHOT);

Was the "|| !m_PredefinedMacros.IsEmpty()" removed on purpose?
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on March 01, 2014, 07:45:17 pm
Two short patches:

1) A fix for tilde operator: make sure if it's really a destructor function, or just a variable with the bitwise NOT operation.
Code: [Select]
Index: src/plugins/codecompletion/codecompletion.cpp
===================================================================
--- src/plugins/codecompletion/codecompletion.cpp (revision 9271)
+++ src/plugins/codecompletion/codecompletion.cpp (working copy)
@@ -249,8 +249,9 @@
                 if (!word.IsEmpty())
                 {
                     NameUnderCursor.Clear();
-                    if (GetLastNonWhitespaceChar(control, start) == _T('~'))
-                        NameUnderCursor << _T('~');
+                    //FIX(huki), don't prepend '~' to search term (could be bitwise operator)
+                    //if (GetLastNonWhitespaceChar(control, start) == _T('~'))
+                    //    NameUnderCursor << _T('~');
                     NameUnderCursor << word;
                     ReturnValue = true;
                     IsInclude = false;
@@ -2182,8 +2233,9 @@
     const int startPos = editor->GetControl()->WordStartPosition(pos, true);
     const int endPos   = editor->GetControl()->WordEndPosition(pos, true);
     wxString target;
+    bool isDestructor = false;  //NOTE(huki), don't prepend '~' in cases of bitwise operator: eg: flag &= ~bits; and use "go to decl" on 'bits'.
     if (CodeCompletionHelper::GetLastNonWhitespaceChar(editor->GetControl(), startPos) == _T('~'))
-        target << _T('~');
+        isDestructor = true; //target << _T('~');
     target << editor->GetControl()->GetTextRange(startPos, endPos);
     if (target.IsEmpty())
         return;
@@ -2201,7 +2253,7 @@
     CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)
 
     // special handle destructor function
-    if (target[0] == _T('~'))
+    if (isDestructor)   //if (target[0] == _T('~'))
     {
         TokenIdxSet tmp = result;
         result.clear();
@@ -2211,7 +2263,7 @@
             const Token* token = tree->at(*it);
             if (token && token->m_TokenKind == tkClass)
             {
-                token = tree->at(tree->TokenExists(target, token->m_Index, tkDestructor));
+                token = tree->at(tree->TokenExists(_T("~") + target, token->m_Index, tkDestructor));
                 if (token)
                     result.insert(token->m_Index);
             }

2) "Go to implementation" fails on constructor declaration. Upon "go to impl", the old code assumes it's a constructor only if the ':' prefix is found. This means user can't go to the ctor implementation from the class declaration for example. i.e.,
Code: [Select]
class CodeCompletion
{
    [...]
   
    /** Constructor */
    CodeCompletion(); <-- Use "go to implementation" on this line

    [...]
}

Fix:
Code: [Select]
Index: src/plugins/codecompletion/codecompletion.cpp
===================================================================
--- src/plugins/codecompletion/codecompletion.cpp (revision 9271)
+++ src/plugins/codecompletion/codecompletion.cpp (working copy)
@@ -2232,8 +2284,9 @@
         }
         if (isClassOrConstructor)
         {
-            const bool isConstructor = CodeCompletionHelper::GetNextNonWhitespaceChar(editor->GetControl(), endPos)   == _T('(')
-                                    && CodeCompletionHelper::GetLastNonWhitespaceChar(editor->GetControl(), startPos) == _T(':');
+            //NOTE(huki), Assume ctor if isImpl (user may want to go to ctor implementation from the class declaration).
+            const bool isConstructor = CodeCompletionHelper::GetNextNonWhitespaceChar(editor->GetControl(), endPos) == _T('(')
+                                    && (isImpl || CodeCompletionHelper::GetLastNonWhitespaceChar(editor->GetControl(), startPos) == _T(':'));
             for (TokenIdxSet::const_iterator it = result.begin(); it != result.end();)
             {
                 const Token* token = tree->at(*it);
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on March 02, 2014, 02:40:13 am
...
Thanks for the commit.. just one question about this change in Parser::OnAllThreadsDone()
Code: [Select]
Index: src/plugins/codecompletion/parser/parser.cpp
===================================================================
--- src/plugins/codecompletion/parser/parser.cpp (revision 9664)
+++ src/plugins/codecompletion/parser/parser.cpp (revision 9665)
@@ -931,9 +888,7 @@
 
     // Do next task
     if (   !m_PoolTask.empty()
-        || !m_BatchParseFiles.empty()
-        || !m_PriorityHeaders.empty()
-        || !m_PredefinedMacros.IsEmpty() )
+        || !m_BatchParseFiles.empty() )
     {
         TRACE(_T("Parser::OnAllThreadsDone(): Still some tasks left, starting m_BatchTimer."));
         m_BatchTimer.Start(ParserCommon::PARSER_BATCHPARSE_TIMER_RUN_IMMEDIATELY, wxTIMER_ONE_SHOT);

Was the "|| !m_PredefinedMacros.IsEmpty()" removed on purpose?

Oh, it is a bug, fixed in trunk now, thanks.
Title: Re: Several improvements to Code Completion plugin
Post by: MortenMacFly on March 02, 2014, 07:30:49 am
2) "Go to implementation" fails on constructor declaration. Upon "go to impl", the old code assumes it's a constructor only if the ':' prefix is found. This means user can't go to the ctor implementation from the class declaration for example. i.e.,
[...]
I think I recall a patch floating around that deals with this already...
@ollydbg: Was it yours? It introduced a special method to handle constructors ... somewhere ... ???
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on March 02, 2014, 03:06:02 pm
2) "Go to implementation" fails on constructor declaration. Upon "go to impl", the old code assumes it's a constructor only if the ':' prefix is found. This means user can't go to the ctor implementation from the class declaration for example. i.e.,
[...]
I think I recall a patch floating around that deals with this already...
@ollydbg: Was it yours? It introduced a special method to handle constructors ... somewhere ... ???
Yes, I have one, I just search our forum for several minutes, and finally found one: Re: Find Declaration of constructor doesn't work (http://forums.codeblocks.org/index.php/topic,13753.msg129197.html#msg129197).
Title: Re: Several improvements to Code Completion plugin
Post by: MortenMacFly on March 02, 2014, 03:42:15 pm
Yes, I have one, I just search our forum for several minutes, and finally found one: Re: Find Declaration of constructor doesn't work (http://forums.codeblocks.org/index.php/topic,13753.msg129197.html#msg129197).
Yes, thats the one I recall. Well - if Hukis patch provides the same results I would vote for that instead, because it looks a bit simpler.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on April 30, 2014, 06:14:04 pm
Hi again.. I've made some corrections to my last patch, so I'm re-posting it.

In case a token is prefixed with the ~ operator, and the user performs "go to decl / impl" on it, we just assume it's a destructor and there is no fallback if there is no such destructor. So the operation would fail in case of normal variables. eg:
Code: [Select]
int token, result;

[...]

result = ~token;
          ^^^^^------------ right-click on 'token' and use go to decl / impl

My patch makes the following changes:
- Don't display the tilde in the right-click context menu, eg., Find declaration of: 'token' rather than Find declaration of: '~token'. (i.e., don't prepend ~ to "NameUnderCursor" string)
- Also don't display the tilde in "symbol not found" messages after go to decl / impl (i.e., don't prepend ~ to "target" string).
- If the tilde prefix is there, first look for a class destructor as usual but if one is not found, fallback to accept any variable.

Code: [Select]
Index: src/plugins/codecompletion/codecompletion.cpp
===================================================================
--- src/plugins/codecompletion/codecompletion.cpp (revision 9271)
+++ src/plugins/codecompletion/codecompletion.cpp (working copy)
@@ -249,8 +249,9 @@
                 if (!word.IsEmpty())
                 {
                     NameUnderCursor.Clear();
-                    if (GetLastNonWhitespaceChar(control, start) == _T('~'))
-                        NameUnderCursor << _T('~');
+                    //FIX(huki), don't prepend '~' to NameUnderCursor (only used for displaying in right-click popup menu)
+                    //if (GetLastNonWhitespaceChar(control, start) == _T('~'))
+                    //    NameUnderCursor << _T('~');
                     NameUnderCursor << word;
                     ReturnValue = true;
                     IsInclude = false;
@@ -2182,8 +2233,9 @@
     const int startPos = editor->GetControl()->WordStartPosition(pos, true);
     const int endPos   = editor->GetControl()->WordEndPosition(pos, true);
     wxString target;
+    bool isDestructor = false;  //NOTE(huki), don't prepend '~' to target, use isDestructor flag instead
     if (CodeCompletionHelper::GetLastNonWhitespaceChar(editor->GetControl(), startPos) == _T('~'))
-        target << _T('~');
+        isDestructor = true; //target << _T('~');
     target << editor->GetControl()->GetTextRange(startPos, endPos);
     if (target.IsEmpty())
         return;
@@ -2201,7 +2253,7 @@
     CC_LOCKER_TRACK_TT_MTX_LOCK(s_TokenTreeMutex)
 
     // special handle destructor function
-    if (target[0] == _T('~'))
+    if (isDestructor)   //if (target[0] == _T('~'))
     {
         TokenIdxSet tmp = result;
         result.clear();
@@ -2211,11 +2263,15 @@
             const Token* token = tree->at(*it);
             if (token && token->m_TokenKind == tkClass)
             {
-                token = tree->at(tree->TokenExists(target, token->m_Index, tkDestructor));
+                token = tree->at(tree->TokenExists(_T("~") + target, token->m_Index, tkDestructor));
                 if (token)
                     result.insert(token->m_Index);
             }
         }
+
+        // no destructor found, but could be a variable.
+        if (result.empty())
+            result = tmp;
     }
     // special handle constructor function
     else

Let me know if it's ok for commit.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on April 30, 2014, 06:27:30 pm
@ollydbg: Also, I think you can go ahead and commit your patch (http://forums.codeblocks.org/index.php/topic,13753.msg129197.html#msg129197) for constructors "go to decl".
From my review and testing it does the following:
- Better handling of "go to decl"  (and also "go to impl") with class constructors.
- In case of function-like usage of a class token (eg, AAA() where AAA is a valid class), go to constructor decl / impl. If none is found, go to the class decl as fallback.
- In case of normal usage of class token, go to class decl as usual.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on May 01, 2014, 12:35:32 am
@ollydbg: Also, I think you can go ahead and commit your patch (http://forums.codeblocks.org/index.php/topic,13753.msg129197.html#msg129197) for constructors "go to decl".
From my review and testing it does the following:
- Better handling of "go to decl"  (and also "go to impl") with class constructors.
- In case of function-like usage of a class token (eg, AAA() where AAA is a valid class), go to constructor decl / impl. If none is found, go to the class decl as fallback.
- In case of normal usage of class token, go to class decl as usual.

Hi, Huki, thanks, the mentioned patch is already in trunk, see: Re: Find Declaration of constructor doesn't work (http://forums.codeblocks.org/index.php/topic,13753.msg131321.html#msg131321)

Hi again.. I've made some corrections to my last patch, so I'm re-posting it.

In case a token is prefixed with the ~ operator, and the user performs "go to decl / impl" on it, we just assume it's a destructor and there is no fallback if there is no such destructor. So the operation would fail in case of normal variables. eg:
Code: [Select]
int token, result;

[...]

result = ~token;
          ^^^^^------------ right-click on 'token' and use go to decl / impl

My patch makes the following changes:
- Don't display the tilde in the right-click context menu, eg., Find declaration of: 'token' rather than Find declaration of: '~token'. (i.e., don't prepend ~ to "NameUnderCursor" string)
- Also don't display the tilde in "symbol not found" messages after go to decl / impl (i.e., don't prepend ~ to "target" string).
- If the tilde prefix is there, first look for a class destructor as usual but if one is not found, fallback to accept any variable.
...
...
...
Let me know if it's ok for commit.

I will check your patches in the following days. Thanks.

Title: Re: Several improvements to Code Completion plugin
Post by: Huki on June 16, 2014, 12:56:18 am
Thanks for commiting the tilde patch.

A small bugfix in nativeparser.cpp, NativeParser::ParseLocalBlock():
I think it's only supposed to be run for function blocks, but it's actually run for any kind of code block (classes, etc). So for example if the user clicks on a class declaration, the entire class block will be parsed as if it's a local block and several token info (such as the line index) will be overwritten. To fix it I've added a check, here's the patch:

Code: [Select]
Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -1877,7 +1892,8 @@
 
         CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
 
-        if (!parent)
+        //FIX(huki), not for tkClass, etc.
+        if (!parent || !(parent->m_TokenKind & tkAnyFunction))
             return false;
     }
 
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on June 16, 2014, 06:52:50 am
Thanks for commiting the tilde patch.

A small bugfix in nativeparser.cpp, NativeParser::ParseLocalBlock():
I think it's only supposed to be run for function blocks, but it's actually run for any kind of code block (classes, etc). So for example if the user clicks on a class declaration, the entire class block will be parsed as if it's a local block and several token info (such as the line index) will be overwritten. To fix it I've added a check, here's the patch:

Code: [Select]
Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -1877,7 +1892,8 @@
 
         CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
 
-        if (!parent)
+        //FIX(huki), not for tkClass, etc.
+        if (!parent || !(parent->m_TokenKind & tkAnyFunction))
             return false;
     }
 

Hi, Huki, thanks, tested and applied in trunk now (revision 9802).
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on June 16, 2014, 06:07:37 pm
A bugfix for crash when trying to cancel the ongoing project parsing (to reparse, quit CB, etc). Easiest way to reproduce is to select "Project -> Reparse current project" twice successively (or once before the initial parsing has finished). It's in fact a mutex deadlock, so here is the patch:
Code: [Select]
Index: src/plugins/codecompletion/parser/parser.cpp
===================================================================
--- src/plugins/codecompletion/parser/parser.cpp (revision 9271)
+++ src/plugins/codecompletion/parser/parser.cpp (working copy)
@@ -385,11 +385,14 @@
 
 Parser::~Parser()
 {
-    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)
+    //FIX: There was a deadlock in TerminateAllThreads() when calling DeleteParser() before
+    // parsing has finished. Moved s_ParserMutex lock below and updated TerminateAllThreads().
 
     DisconnectEvents();
     TerminateAllThreads();
 
+    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)
+
     if (ParserCommon::s_CurrentParser == this)
         ParserCommon::s_CurrentParser = nullptr;
 
@@ -851,6 +867,8 @@
 
 void Parser::TerminateAllThreads()
 {
+    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)
+
     while (!m_PoolTask.empty())
     {
         PTVector& v = m_PoolTask.front();
@@ -859,6 +877,11 @@
         m_PoolTask.pop();
     }
 
+    CC_LOCKER_TRACK_P_MTX_UNLOCK(ParserCommon::s_ParserMutex)
+
+    //NOTE: This should not be locked with s_ParserMutex, otherwise we'll be stuck in an
+    // infinite loop below since the worker thread also enters s_ParserMutex.
+    // In fact cbThreadPool maintains it's own mutex, so m_Pool is probably threadsafe.
     m_Pool.AbortAllTasks();
     while (!m_Pool.Done())
         wxMilliSleep(1);

And another patch to speed up canceling the ongoing parser:
Code: [Select]
Index: src/plugins/codecompletion/nativeparser.cpp
===================================================================
--- src/plugins/codecompletion/nativeparser.cpp (revision 9271)
+++ src/plugins/codecompletion/nativeparser.cpp (working copy)
@@ -616,14 +616,20 @@
 
     if (m_ParsedProjects.empty())
     {
-        if (it->second == m_Parser)
-          SetParser(m_TempParser); // Also updates class browser
-
         wxString log(F(_("NativeParser::DeleteParser(): Deleting parser for project '%s'!"), prj.wx_str()));
         CCLogger::Get()->Log(log);
         CCLogger::Get()->DebugLog(log);
 
         delete it->second;
+
+        //NOTE: Moved here as RemoveLastFunctionChildren() in SetParser() takes quite a while if
+        // parsing hasn't finished yet. Just delete the entire parser (above), then change to m_TempParser.
+        if (it->second == m_Parser)
+        {
+            m_Parser = nullptr;
+            SetParser(m_TempParser); // Also updates class browser
+        }
+
         m_ParserList.erase(it);
 
         return true;
@@ -1253,7 +1275,9 @@
     if (m_Parser == parser)
         return;
 
-    RemoveLastFunctionChildren(m_Parser->GetTokenTree(), m_LastFuncTokenIdx);
+    if (m_Parser)   //ADD, in case parser deleted before calling SetParser().
+        RemoveLastFunctionChildren(m_Parser->GetTokenTree(), m_LastFuncTokenIdx);
+
     InitCCSearchVariables();
     m_Parser = parser;
 
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on June 17, 2014, 08:34:23 am
Hi, Huki, thanks, would you mind to update your local copy of C::B to the latest SVN? I have some tiny issue to apply your patch, though I can manually applied it, but it looks like some of the code I have already changed.

In the latest revision (rev9807), I have already add a comments here in the function.
Code: [Select]
void Parser::TerminateAllThreads()
{
    // FIXME (ollydbg#1#): Do we need to use a mutex to protect the m_PoolTask accessing?
    while (!m_PoolTask.empty())
    {
        PTVector& v = m_PoolTask.front();
        for (PTVector::iterator it = v.begin(); it != v.end(); ++it)
            delete *it;
        m_PoolTask.pop();
    }
Look at the FIXME.

BTW, in one testing branch of my local copy, I have try to remove all the lockers in CC (locker of the Parser and locker of the TokenTree), also, I remove the m_PoolTask, it was introduced to handle the priority header files, but now, we can already recursive to parse the included files, so I don't think m_PoolTask is needed any longer. Also, my local branch has many other issues I need to fix, so it will take a long time before I publish the patches  ;)


EDIT:
A bugfix for crash when trying to cancel the ongoing project parsing (to reparse, quit CB, etc). Easiest way to reproduce is to select "Project -> Reparse current project" twice successively (or once before the initial parsing has finished). It's in fact a mutex deadlock, so here is the patch:
...
I try to reproduce this bug, but sorry I can't reproduce the dead lock in rev9807, under Window XP. I try the two method:
method one:
Code: [Select]
1, load a project, so the parsing started.
2, right click on the project, and select Peparse current project
3, no crash happened

method two:
Code: [Select]
1, right click on the project, and select Peparse current project
2, right click on the project, and select Peparse current project again
3, no crash happened

Can you give some more detailed explanation? Thanks.
Title: Re: Several improvements to Code Completion plugin
Post by: White-Tiger on June 17, 2014, 10:26:41 am
I guess he fixed what I were talking about back here: http://forums.codeblocks.org/index.php/topic,18044.msg124560.html#msg124560
You never fixed it IIRC... probably because you couldn't reproduce it.. (I couldn't get a proper stack trace)

But it's been a while since I had a freeze like that... I'm carefully with closing projects :P (and I've got a new PC now... so it's even more unlikely)
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on June 17, 2014, 10:48:11 am
I guess he fixed what I were talking about back here: http://forums.codeblocks.org/index.php/topic,18044.msg124560.html#msg124560
You never fixed it IIRC... probably because you couldn't reproduce it.. (I couldn't get a proper stack trace)

But it's been a while since I had a freeze like that... I'm carefully with closing projects :P (and I've got a new PC now... so it's even more unlikely)
Thanks for the remind, yes, CC sometimes just hangs on my system too (I use a notepad which bought in 2009, CC hangs when loading a large project, it always hang at the first time I load the project, not the second time), It looks like other devs don't have such hang issue.  :(
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on June 17, 2014, 11:27:27 am
The parser handling is a mass, see a belief draft diagram
Code: [Select]
CodeCompletion receive Workspace changed (mostly because project loaded finishes)
->NativeParser::CreateParser for the active project
  ->new Parser
  ->DoFullParsing
    ->Fill Parser's macro definition(from compiler and from project setting)
    ->Fill Parser's file list need to parse, kick the batch timer
   
Parser receive batch timer event
->new ParserThreadedTask (this task will executed in thread pool)
->send Parse Start Event!

ParserThreadedTask is executed
->parse the macro definition
->new ParserThread for each file in file list
->put ParserThread in thread queue

Thread pool finish running tasks:
->do one of below
  *CASE1: if thread queue is not empty, copy to thread pool, run
  *CASE2: if file list is not empty, kick the batch timer
  *CASE3: if macro definition is not empty, kick the batch timer
  *CASE4: non of the above cases, send Parse Finish Event!

Title: Re: Several improvements to Code Completion plugin
Post by: White-Tiger on June 17, 2014, 05:11:03 pm
non the less.. I couldn't reproduce it with my current system :P
All I've got when I close a project while the parser is running, or when forcing a reparse, is just a small hang... so about 3 seconds or more.. (tested with Code::Blocks project itself)

I've also tried it with CB only 2 cores assigned (that's what my old system was, Dualcore) So maybe you've fixed the problems I had... maybe my new system makes the difference, idk.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on June 17, 2014, 07:01:57 pm
Hi, Huki, thanks, would you mind to update your local copy of C::B to the latest SVN? I have some tiny issue to apply your patch, though I can manually applied it, but it looks like some of the code I have already changed.
Oh, sorry about that. I'm in fact planning on moving some of my projects to git very soon and if everything goes well I'll be updating to obf's repo here https://github.com/obfuscated/codeblocks_sf. In the meantime I'm trying to get some of the small patches in (just a few more actually).

BTW, in one testing branch of my local copy, I have try to remove all the lockers in CC (locker of the Parser and locker of the TokenTree), also, I remove the m_PoolTask, it was introduced to handle the priority header files, but now, we can already recursive to parse the included files, so I don't think m_PoolTask is needed any longer. Also, my local branch has many other issues I need to fix, so it will take a long time before I publish the patches
The CC code indeed needs some cleanup since we don't use the priority headers. It's good if the m_PoolTask can be removed, and while it's here I think it does needs to be protected since the last time I checked it was accessed from different threads..

Btw, though I'm using an old revision I do check the latest revisions of files which I've modified, to make sure my fix is still relevant. For this one (i.e., the mutex deadlock) I noticed no changes so I was quite sure the bug still exists in the newer revisions. It's very strange that you and White-Tiger are unable to reproduce it. Let me explain:

When calling DeleteParser() we lock the parser mutex before calling TerminateAllThreads():
Code: [Select]
Parser::~Parser()
{
    CC_LOCKER_TRACK_P_MTX_LOCK(ParserCommon::s_ParserMutex)

    DisconnectEvents();
    TerminateAllThreads();

    [...]

    CC_LOCKER_TRACK_P_MTX_UNLOCK(ParserCommon::s_ParserMutex)
}

In TerminateAllThreads(), we send the request to abort all threads, and wait in a while loop till all threads have aborted:
Code: [Select]
   m_Pool.AbortAllTasks();
    while (!m_Pool.Done())
        wxMilliSleep(1);

At this time if one of the worker threads has already been executed, you can see in the Execute call that it would enter and leave the parser mutex many times during the course of the call:
Code: Text
  1. int ParserThreadedTask::Execute()
  2. {
  3.     TRACE(_T("ParserThreadedTask::Execute(): Enter"));
  4.     if (!m_Parser) return 0;
  5.  
  6.     CC_LOCKER_TRACK_P_MTX_LOCK(m_ParserMutex)
  7.  
  8.     [...]
  9.  
  10.     CC_LOCKER_TRACK_P_MTX_UNLOCK(m_ParserMutex);
  11.  
  12.     [...]
  13.  
  14.     CC_LOCKER_TRACK_P_MTX_LOCK(m_ParserMutex)
  15.  
  16.     [...]
  17.  
  18.  

And if Execute() is waiting to enter this mutex while we are doing the while loop in TerminateAllThreads(), we would never return. FYI, I had confirmed this condition when checking this bug under debugger.

Now, the m_Pool object happens to keep it's own mutex to protect calls to it from multiple threads, so there doesn't seem to be any reason to use the parser mutex around the m_Pool object. So in my patch I had removed the lock around the while loop, and also around the DisconnectEvents() call in ~Parser().
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on June 18, 2014, 04:32:59 am
Hi, Huki, thanks for the detailed explanation, I debugged this issue, the interesting thing is that the ParserthreadedTask runs very fast after creating the parser, so when I close the parser, I always see ParserthreadedTask already finishes its job.  :)

I just find a way to reproduce the dead lock, I add a long sleep. (during the long sleep, I just close the project)
Code: [Select]
src/plugins/codecompletion/parser/parserthreadedtask.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/plugins/codecompletion/parser/parserthreadedtask.cpp b/src/plugins/codecompletion/parser/parserthreadedtask.cpp
index d3c98b6..ac11852 100644
--- a/src/plugins/codecompletion/parser/parserthreadedtask.cpp
+++ b/src/plugins/codecompletion/parser/parserthreadedtask.cpp
@@ -66,6 +66,8 @@ int ParserThreadedTask::Execute()
     TRACE(_T("ParserThreadedTask::Execute(): Enter"));
     if (!m_Parser) return 0;
 
+    wxSleep(15);
+
     CC_LOCKER_TRACK_P_MTX_LOCK(m_ParserMutex)
 
     wxString   preDefs(m_Parser->m_PredefinedMacros);
Now, I see the dead lock. I will commit your patch later today. Thanks.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on June 18, 2014, 04:35:39 am
non the less.. I couldn't reproduce it with my current system :P
All I've got when I close a project while the parser is running, or when forcing a reparse, is just a small hang... so about 3 seconds or more.. (tested with Code::Blocks project itself)

I've also tried it with CB only 2 cores assigned (that's what my old system was, Dualcore) So maybe you've fixed the problems I had... maybe my new system makes the difference, idk.
The issue reported by Huki is located, see my previous post, so I will commit his patch.

Quote
is just a small hang... so about 3 seconds or more
I also noticed this 3 seconds hang, not sure what cause this hang.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on June 18, 2014, 07:35:19 am
@Huki, the two patches in Re: Several improvements to Code Completion plugin (http://forums.codeblocks.org/index.php/topic,18315.msg132346.html#msg132346) are in trunk now (rev9809 and rev9810), thanks for contribution.
BTW: I add some comments.  ;)
Title: Re: Several improvements to Code Completion plugin
Post by: Huki 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: [Select]
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: [Select]
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);
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg 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: [Select]
   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. :)
Title: Re: Several improvements to Code Completion plugin
Post by: Huki 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: [Select]
  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.
Title: Re:
Post by: MortenMacFly on June 21, 2014, 07:12:54 am
Still, please always try to provide patches against trunk HEAD. Everything else is error prone.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg 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.


Title: Re: Several improvements to Code Completion plugin
Post by: Huki 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.. :)
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg 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".
Title: Re: Several improvements to Code Completion plugin
Post by: Huki 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.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg 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.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki 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. :)
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg 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.

Title: Re: Several improvements to Code Completion plugin
Post by: Huki 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 (http://forums.codeblocks.org/index.php/topic,18315.msg125579.html#msg125579) 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: [Select]
    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.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki 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: [Select]
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: [Select]
 // 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.

Title: Re: Several improvements to Code Completion plugin
Post by: Huki 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: [Select]
#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: [Select]
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
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg 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: [Select]
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
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on June 26, 2014, 08:22:19 pm
Thanks for the commit. 2 patches for parsing bugs:

1) Don't skip successive opening or closing brackets. For example, see any occurrence of "GetConfigManager(_T("code_completion"))->XXX" in CodeCompletion.cpp,
Code: [Select]
void CodeCompletion::UpdateToolBar()
{
    bool showScope = Manager::Get()->GetConfigManager(_T("code_completion"))->ReadBool(_T("/scope_filter"), true);
                                                                              ^^^^^^^^________________________________ // cannot use CC on ReadBool
    [...]

Fix:
Code: [Select]
From dcc4ea033b398329915a900faada2d1254261313 Mon Sep 17 00:00:00 2001
From: huki <[email protected]>
Date: Tue, 24 Jun 2014 08:11:28 +0530
Subject: [PATCH 04/30] CC: fix parser successive brackets

---
 src/plugins/codecompletion/nativeparser_base.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/plugins/codecompletion/nativeparser_base.cpp b/src/plugins/codecompletion/nativeparser_base.cpp
index 89e252c..3c5cf75 100644
--- a/src/plugins/codecompletion/nativeparser_base.cpp
+++ b/src/plugins/codecompletion/nativeparser_base.cpp
@@ -561,6 +561,9 @@ unsigned int NativeParserBase::FindCCTokenStart(const wxString& line)
 
                     if (IsClosingBracket(startAt, line))
                         ++nest;
+                    //NOTE: do not skip successive opening brackets.
+                    if (IsOpeningBracket(startAt, line))
+                        --nest;
                 }
 
                 startAt = BeforeToken(startAt, line);
@@ -650,6 +653,15 @@ wxString NativeParserBase::GetNextCCToken(const wxString& line,
 
             if (IsOpeningBracket(startAt, line))
                 ++nest;
+            //NOTE: do not skip successive closing brackets. Eg,
+            // "GetConfigManager(_T("code_completion"))->ReadBool"
+            //                                        ^
+            if (IsClosingBracket(startAt, line))
+            {
+                --nest;
+                if (nest == 0)
+                    ++startAt;
+            }
         }
     }
     if (IsOperatorBegin(startAt, line))
--
1.9.4.msysgit.0


2) In DoParse(), when parsing "else" we try to eat the arguments. It breaks support for "else-if" (we will end up eating the "if"), and anyway there is no need to skip anything after "else".
Code: [Select]
From 900b6df9304e30455438c362f2757c88bd690494 Mon Sep 17 00:00:00 2001
From: huki <[email protected]>
Date: Tue, 24 Jun 2014 09:20:07 +0530
Subject: [PATCH 26/30] CC: parser: fixed 'else-if'

---
 src/plugins/codecompletion/parser/parserthread.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/plugins/codecompletion/parser/parserthread.cpp b/src/plugins/codecompletion/parser/parserthread.cpp
index f87b1b7..938a134 100644
--- a/src/plugins/codecompletion/parser/parserthread.cpp
+++ b/src/plugins/codecompletion/parser/parserthread.cpp
@@ -714,8 +714,9 @@ void ParserThread::DoParse()
             {
                 if (!m_Options.useBuffer || m_Options.bufferSkipBlocks)
                     SkipToOneOfChars(ParserConsts::semicolonclbrace, true, true);
-                else
-                    m_Tokenizer.GetToken(); // skip arguments
+                //NOTE: this breaks support for 'else if' !
+                // else
+                //     m_Tokenizer.GetToken(); // skip arguments
                 m_Str.Clear();
             }
             else if (token == ParserConsts::kw_enum)
--
1.9.4.msysgit.0

Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on July 16, 2014, 08:51:14 am
Thanks for the commit. 2 patches for parsing bugs:
...snip...
Hi, sorry for the late reply, I'm busy those days.
I tested and committed the two patches(rev9846 and rev9847), thanks for your contribution. ;)
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on August 24, 2014, 01:01:01 pm
Thanks for the commit. 2 patches for parsing bugs:
...snip...
Hi, sorry for the late reply, I'm busy those days.
I tested and committed the two patches(rev9846 and rev9847), thanks for your contribution. ;)
Hi, thanks for the commit.

A small update to the undo token behavior fix (http://forums.codeblocks.org/index.php/topic,18315.msg132555.html#msg132555).
There was some code I added to CalcConditionExpression() by mistake (assuming it is called directly inside SkipUnwanted()). See my comments below:
[...] A preprocessor condition is hit in SkipUnwanted() and we run CalcConditionExpression(). [...] We need to reset Undo and Saved positions to the proper place after CalcConditionExpression() (because it's not only expanding the macro but also parsing it fully, so we need to reset right after the preprocessor line has ended).

In fact the function which is called in SkipUnwanted() is HandleConditionPreprocessor(), that's where we should reset the undo and savetoken values. If we reset them in CalcConditionExpression() we will always reset to the start of the first conditional block even if it is false.
Code: [Select]
#if 0| <---- adding the code in CalcConditionExpression() will reset undo vars to here which is wrong
// inactive code
#else| <---- adding the code in HandleConditionPreprocessor() will reset undo vars to here
// active code
#endif

I've also added a small note to avoid recursive call to PeekToken() (which shouldn't really happen).


Here's the patch:
Code: [Select]
From fd5af3421e85d9eff782c63ab07736cadf04d45c Mon Sep 17 00:00:00 2001
From: huki <[email protected]>
Date: Fri, 4 Jul 2014 02:32:16 +0530
Subject: CC: update to undo token fix

---
 src/plugins/codecompletion/parser/tokenizer.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/plugins/codecompletion/parser/tokenizer.cpp b/src/plugins/codecompletion/parser/tokenizer.cpp
index 2bfb113..4a8f0cb 100644
--- a/src/plugins/codecompletion/parser/tokenizer.cpp
+++ b/src/plugins/codecompletion/parser/tokenizer.cpp
@@ -1043,7 +1043,6 @@ wxString Tokenizer::PeekToken()
 {
     if (!m_PeekAvailable)
     {
-        m_PeekAvailable = true;
         //NOTE: The m_Saved... vars will be reset to the correct position
         // as necessary when a ReplaceBufferText() is done.
         m_SavedTokenIndex   = m_TokenIndex;
@@ -1055,6 +1054,7 @@ wxString Tokenizer::PeekToken()
         else
             m_PeekToken.Clear();
 
+        m_PeekAvailable     = true; //NOTE: Moved after DoGetToken() to avoid recursive PeekToken() calls.
         m_PeekTokenIndex    = m_TokenIndex;
         m_PeekLineNumber    = m_LineNumber;
         m_PeekNestLevel     = m_NestLevel;
@@ -1407,11 +1407,6 @@ bool Tokenizer::CalcConditionExpression(bool checkResult)
     // reset tokenizer's functionality
     m_State = oldState;
 
-    // reset undo token
-    m_SavedTokenIndex   = m_UndoTokenIndex = m_TokenIndex;
-    m_SavedLineNumber   = m_UndoLineNumber = m_LineNumber;
-    m_SavedNestingLevel = m_UndoNestLevel  = m_NestLevel;
-
     exp.ConvertInfixToPostfix();
     if (exp.CalcPostfix())
     {
@@ -1697,6 +1692,11 @@ void Tokenizer::HandleConditionPreprocessor(const PreprocessorType type)
         default:
             break;
     }
+
+    // reset undo token
+    m_SavedTokenIndex   = m_UndoTokenIndex = m_TokenIndex;
+    m_SavedLineNumber   = m_UndoLineNumber = m_LineNumber;
+    m_SavedNestingLevel = m_UndoNestLevel  = m_NestLevel;
 }
 
 void Tokenizer::SplitArguments(wxArrayString& results)
@@ -1826,6 +1826,7 @@ bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken)
     {
         m_PeekAvailable = false;
         PeekToken(); // NOTE (ollydbg#1#): chance of recursive call of this function
+                     // NOTE (huki): updated PeekToken() to prevent unnecessary recursive call
     }
 
     return true;
--
1.9.4.msysgit.0

Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on August 27, 2014, 05:54:59 am
Hi, Huki, thanks for the contribution, today, I looked at you patch, and I think that

Code: [Select]
    // Update the peek token
    if (m_PeekAvailable && updatePeekToken)
    {
        m_PeekAvailable = false;
        PeekToken(); // NOTE (ollydbg#1#): chance of recursive call of this function
                     // NOTE (huki): updated PeekToken() to prevent unnecessary recursive call
    }
I think here, with your patch, the m_PeekAvailable should always be false in the if condition, right?
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on August 27, 2014, 10:01:55 am
Hi, Huki, thanks for the contribution, today, I looked at you patch, and I think that

Code: [Select]
   // Update the peek token
    if (m_PeekAvailable && updatePeekToken)
    {
        m_PeekAvailable = false;
        PeekToken(); // NOTE (ollydbg#1#): chance of recursive call of this function
                     // NOTE (huki): updated PeekToken() to prevent unnecessary recursive call
    }
I think here, with your patch, the m_PeekAvailable should always be false in the if condition, right?
Oh, I was wrong, I just debug the code and found that the function bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken) will be called from void ParserThread::HandleMacroExpansion(int id, const wxString &peek), which is called from DoParse() function.

I understand you idea, that is the function Tokenizer::ReplaceBufferText() can also be called inside the PeekToken() function.
The old way (without your patch) of PeekToken() could be:
Code: [Select]
first set the flag: m_PeekAvailable = true;
some function will call ReplaceBufferText();
Thus, the condition "if (m_PeekAvailable && updatePeekToken)" is true, and we comes with a recursive call to PeekToken().

Now, in your patch, you set the flag a bit later, which are something like:
Code: [Select]
some function will call ReplaceBufferText();
set the flag: m_PeekAvailable = true;
Here, the if condition is always false, so we can avoid the recursive call to PeekToken().

Question: what happens about old way (without your patch), do we enter a infinite recursive call to PeekToken()?

Title: Re: Several improvements to Code Completion plugin
Post by: Huki on August 27, 2014, 11:45:51 am
Yes, the m_PeekAvailable may be true in some cases, eg:
Code: [Select]
In DoParse():
    PeekToken(); // sets m_PeekAvailable to true
    [...]
    ReplaceBufferText(); // m_PeekAvailable is true, so call PeekToken() and update m_PeekToken

My patch makes sure that m_PeekAvailable is not true if we are already inside PeekToken(), so a useless recursive call is avoided. i.e.,
Code: [Select]
PeekToken():
  -> DoGetToken():
    -> ReplaceMacro():
      -> ReplaceBufferText(); // no need for recursive call to PeekToken() as we are already going to
                              // update m_PeekToken after returning from DoGetToken()
      -> DoGetToken(); // recurse DoGetToken()

  -> m_PeekAvailable = true; // got m_PeekToken, now set this to true

Question: what happens about old way (without your patch), do we enter a infinite recursive call to PeekToken()?
I don't think there is an infinite recursive call, but generally PeekToken and GetToken are not supposed to recurse, because they update class variables (GetToken() updates Undo__ vars and PeekToken() updates Save__ vars), so a recursive call would overwrite these values, which is bad. So we should first get the peek token (i.e., m_PeekToken = DoGetToken(); ), then set m_PeekAvailable = true;
Title: Re: Several improvements to Code Completion plugin
Post by: SteelRat on August 27, 2014, 07:28:51 pm
Code completion ignores code like this:
Code: [Select]
#if defined(TOUCHSCREEN)
void InputManager::ConvertTouch(sf::Event& event)
{
...
}
#endif

Parser does not understand "final" correctly?
An checkboxes in "Editor->Code complition->C++ parser" "Parse preproceccor..." "Parse macros..." are not saving their state. They are checked always.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on August 28, 2014, 03:09:50 am
Yes, the m_PeekAvailable may be true in some cases, eg:
Code: [Select]
In DoParse():
    PeekToken(); // sets m_PeekAvailable to true
    [...]
    ReplaceBufferText(); // m_PeekAvailable is true, so call PeekToken() and update m_PeekToken

My patch makes sure that m_PeekAvailable is not true if we are already inside PeekToken(), so a useless recursive call is avoided. i.e.,
Code: [Select]
PeekToken():
  -> DoGetToken():
    -> ReplaceMacro():
      -> ReplaceBufferText(); // no need for recursive call to PeekToken() as we are already going to
                              // update m_PeekToken after returning from DoGetToken()
      -> DoGetToken(); // recurse DoGetToken()

  -> m_PeekAvailable = true; // got m_PeekToken, now set this to true

Question: what happens about old way (without your patch), do we enter a infinite recursive call to PeekToken()?
I don't think there is an infinite recursive call, but generally PeekToken and GetToken are not supposed to recurse, because they update class variables (GetToken() updates Undo__ vars and PeekToken() updates Save__ vars), so a recursive call would overwrite these values, which is bad. So we should first get the peek token (i.e., m_PeekToken = DoGetToken(); ), then set m_PeekAvailable = true;

Hi, Huki, very clear explanation. I have committed your patch to trunk now with some comments added. Thank you very much for the contribution. :)
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on August 28, 2014, 03:32:36 am
Code completion ignores code like this:
Code: [Select]
#if defined(TOUCHSCREEN)
void InputManager::ConvertTouch(sf::Event& event)
{
...
}
#endif

I just test a minimal sample code
Code: [Select]
#define TOUCHSCREEN 1
#if defined(TOUCHSCREEN)
void InputManager::ConvertTouch(sf::Event& event)
{

}
#endif
It works fine here, I can see the member function symbols in code completion plugin's symbol browser tree.



Code: [Select]
Parser does not understand "final" correctly?Yeah, you can write a feature request in Sourceforge as I can see.

Quote
An checkboxes in "Editor->Code complition->C++ parser" "Parse preproceccor..." "Parse macros..." are not saving their state. They are checked always.
Works fine here. (WinXP, latest nightly build C::B)
Title: Re: Several improvements to Code Completion plugin
Post by: SteelRat on August 28, 2014, 07:09:51 am
Thanks, I'll try other ways or with new .conf file.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki on August 28, 2014, 08:33:33 pm
Here is a patch that adds support for:
1) Nested unnamed (struct or union) within unnamed: all the members should be invokable from the parent class or struct.
2) Show tooltips for members of unnamed / enum within class invoked directly (also for nested cases).

Patch:
Code: [Select]
From ef154d4ef188d77b10bc7ff55d5bcb54b18d66a2 Mon Sep 17 00:00:00 2001
From: huki <[email protected]>
Date: Tue, 24 Jun 2014 08:15:00 +0530
Subject: CC: Support unnamed or enum within class, recursive

---
 src/plugins/codecompletion/nativeparser_base.cpp | 13 +++++++--
 src/plugins/codecompletion/nativeparser_base.h   | 34 +++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/src/plugins/codecompletion/nativeparser_base.cpp b/src/plugins/codecompletion/nativeparser_base.cpp
index e85b8be..5f04b0c 100644
--- a/src/plugins/codecompletion/nativeparser_base.cpp
+++ b/src/plugins/codecompletion/nativeparser_base.cpp
@@ -1411,7 +1411,14 @@ size_t NativeParserBase::GenerateResultSet(TokenTree*          tree,
                 {
                     const Token* token = tree->at(*it);
                     // check whether its under the parentIdx
-                    if (token && (token->m_ParentIndex == parentIdx))
+                    //NOTE: check for unnamed or enum inside class.
+                    // eg, 'ParserCommon::ParserState::ptCreateParser' should be accessed as 'ParserCommon::ptCreateParser'.
+                    // Here, token is ptCreateParser and parentIdx is ParserCommon, so 'token->m_ParentIndex == parentIdx'
+                    // is false. Now, we iterate over the children of parentIdx and check if any of them is unnamed or enum
+                    // and match with token->m_ParentIndex. Thus if we confirm that 'token' is a child of unnamed or enum
+                    // (i.e., m_ParentIndex), we add the token to result.
+                    if (token && ((token->m_ParentIndex == parentIdx)
+                              || IsChildOfUnnamedOrEnum(tree, token->m_ParentIndex, parentIdx)))
                         result.insert(*it);
 
                     // "result" will become the search scope for the next loop, so
@@ -1437,7 +1444,9 @@ size_t NativeParserBase::GenerateResultSet(TokenTree*          tree,
                                   ancestorIterator != tokenParent->m_Ancestors.end();
                                   ++ancestorIterator )
                             {
-                                if (token->m_ParentIndex == (*ancestorIterator)) //matched
+                                //NOTE: check for unnamed or enum inside class (see note above).
+                                if (token && ((token->m_ParentIndex == (*ancestorIterator)) //matched
+                                          || IsChildOfUnnamedOrEnum(tree, token->m_ParentIndex, (*ancestorIterator))))
                                     result.insert(*it);
                             }
                         }
diff --git a/src/plugins/codecompletion/nativeparser_base.h b/src/plugins/codecompletion/nativeparser_base.h
index 3141e61..da60fea 100644
--- a/src/plugins/codecompletion/nativeparser_base.h
+++ b/src/plugins/codecompletion/nativeparser_base.h
@@ -495,7 +495,12 @@ private:
                 if (    tokenChild
                     && (parent->m_TokenKind == tkClass || tokenChild->m_Scope != tsPrivate) )
                 {
-                    result.insert(*it);
+                    //NOTE: recurse (eg: class A contains struct contains union or enum)
+                    if ( !AddChildrenOfUnnamed(tree, tokenChild, result) )
+                    {
+                        result.insert(*it);
+                        AddChildrenOfEnum(tree, tokenChild, result);
+                    }
                 }
             }
             return true;
@@ -520,6 +525,33 @@ private:
         }
         return false;
     }
+
+    bool IsChildOfUnnamedOrEnum(TokenTree* tree, const int targetIdx, const int parentIdx)
+    {
+        if (targetIdx == parentIdx)
+            return true;
+        if (parentIdx == -1)
+            return false;
+
+        Token* parent = tree->at(parentIdx);
+        if (parent && (parent->m_TokenKind & tkClass))
+        {
+            for (TokenIdxSet::const_iterator it = parent->m_Children.begin();
+                                             it != parent->m_Children.end(); ++it)
+            {
+                Token* token = tree->at(*it);
+                if (token && (((token->m_TokenKind & tkClass)
+                                && (token->m_IsAnonymous == true))
+                             || (token->m_TokenKind & tkEnum)))
+                {
+                    if ((targetIdx == (*it)) || IsChildOfUnnamedOrEnum(tree, targetIdx, (*it)))
+                        return true;
+                }
+            }
+        }
+        return false;
+    }
+
 
     /**  loop on the input Token index set (source), add all its public constructors to output Token index set (dest) */
     void AddConstructors(TokenTree *tree, const TokenIdxSet& source, TokenIdxSet& dest);
--
1.9.4.msysgit.0


Code to test:
Code: [Select]
class ClassA
{
public:
    int a;
    struct A
    {
        int c;
    } z;
    struct
    {
        int s;
        union
        {
            int u;
            short v;
        };
        enum EnumS
        {
            STRUCT_A,
            STRUCT_B,
        };
    };

    enum
    {
        UNNAMED_A,
        UNNAMED_B
    };
    enum EnumA
    {
        ENUM_A,
        ENUM_B
    };
};

// Hovering on following member variables should
// show tooltip: a, s, u, v, all the enums like STRUCT_A, etc.
ClassA::a; // replace 'a' with other members listed above

// However ClassA::c is illegal and should not display tooltip.
ClassA::c; // no tip
ClassA::A::c; // should show tip

// Code completion should show all legal members.
ClassA:: // cursor here

Title: Re: Several improvements to Code Completion plugin
Post by: SteelRat on August 29, 2014, 10:34:53 pm
I just test a minimal sample code
Code: [Select]
#define TOUCHSCREEN 1
#if defined(TOUCHSCREEN)
void InputManager::ConvertTouch(sf::Event& event)
{

}
#endif
It works fine here, I can see the member function symbols in code completion plugin's symbol browser tree.

Here it is, macros highlighting for inactive code is ON, TEST defined in Compiler's global defines. Parser knows about it and about TEST2, but it can't find declaration for TEST2 and TEST3.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on August 30, 2014, 08:11:00 am
Here it is, macros highlighting for inactive code is ON
This option is not related to CC plugin.


Quote
TEST defined in Compiler's global defines. Parser knows about it and about TEST2, but it can't find declaration for TEST2 and TEST3.

Confirm this bug. It looks like CC doesn't collect the macro definitions if it is defined in "Compiler's global defines".

Note that the below code works OK
Code: [Select]
#define TEST

#ifdef TEST
    #define TEST2
#endif // TEST

#ifdef TEST2
    #define TEST3
#endif // TEST2



Title: Re: Several improvements to Code Completion plugin
Post by: MortenMacFly on August 30, 2014, 11:47:55 am
Confirm this bug. It looks like CC doesn't collect the macro definitions if it is defined in "Compiler's global defines".
I've noticed this, too. Its even worse: CC does not honour if you've setup something like "target options only" or alike. Also the order of global compile/project/target settings is not considered which may override #defines. Collecting existing macros could need some improvements.

I have it on my todo list but didn't look into it so far...
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on August 30, 2014, 11:57:05 am
Confirm this bug. It looks like CC doesn't collect the macro definitions if it is defined in "Compiler's global defines".
I've noticed this, too. Its even worse: CC does not honour if you've setup something like "target options only" or alike.
I think CC just collect all the target options, we have discussed this some times ago in this thread, Huki has a patch to reparse the project if an active target changes, but I have some concerns about this option.

Quote
Also the order of global compile/project/target settings is not considered which may override #defines. Collecting existing macros could need some improvements.
We have already a function bool NativeParser::AddProjectDefinedMacros(cbProject* project, ParserBase* parser), which collects macro definitions for a whole project. Adding the global compiler options should be similar.
Title: Re: Several improvements to Code Completion plugin
Post by: SteelRat on August 30, 2014, 03:47:15 pm
And can you create control to define defines :) that are already defined in the compiler? __MSVC__, for example, or ANDROID (__ANDROID__) that is hardcoded in Android's NDK toolchains. There no need to REALLY define them via command line, but just to let CC know that selected compiler has global defines.
Or it can be not the GUI control but part of XML description of compiler - it will be even better.
Title: Re: Several improvements to Code Completion plugin
Post by: MortenMacFly on August 30, 2014, 05:27:19 pm
__MSVC__, for example, or ANDROID (__ANDROID__) that is hardcoded in Android's NDK toolchains.
For MSVC and GCC based compilers we collect these by running the compiler with a NULL file having it output exactly those platform defines. This should work. So this should actually not be an issue.
Title: Re: Several improvements to Code Completion plugin
Post by: oBFusCATed on August 30, 2014, 05:50:40 pm
Does this happen for clang and intel's compiler?
Title: Re: Several improvements to Code Completion plugin
Post by: SteelRat on August 30, 2014, 07:06:00 pm
For MSVC and GCC based compilers we collect these by running the compiler with a NULL file having it output exactly those platform defines. This should work. So this should actually not be an issue.
Well it does not work with Android's GCC.
Title: Re: Several improvements to Code Completion plugin
Post by: MortenMacFly on August 30, 2014, 09:41:17 pm
Does this happen for clang and intel's compiler?
I should check again, but most likely not.
Title: Re: Several improvements to Code Completion plugin
Post by: MortenMacFly on August 30, 2014, 09:42:45 pm
Well it does not work with Android's GCC.
What makes you say so? It works the same way, hence the specific (target-)platform #defines you have to set anyways (best within your project settings).
Title: Re: Several improvements to Code Completion plugin
Post by: SteelRat on August 30, 2014, 10:05:22 pm
Because #ifdef __ANROID__ ... #endif section is grey-inactive, when __ANDROID__ is not defined manually. As you see __ANDROID__ is defined:
Code: [Select]
/arm-linux-androideabi-g++  -dM -E - < /dev/null
#define __DBL_MIN_EXP__ (-1021)
#define __HQ_FBIT__ 15
#define __UINT_LEAST16_MAX__ 65535
#define __ARM_SIZEOF_WCHAR_T 32
#define __ATOMIC_ACQUIRE 2
#define __SFRACT_IBIT__ 0
#define __FLT_MIN__ 1.1754943508222875e-38F
#define __UFRACT_MAX__ 0XFFFFP-16UR
#define __UINT_LEAST8_TYPE__ unsigned char
#define __DQ_FBIT__ 63
#define __INTMAX_C(c) c ## LL
#define __ULFRACT_FBIT__ 32
#define __SACCUM_EPSILON__ 0x1P-7HK
#define __CHAR_BIT__ 8
#define __USQ_IBIT__ 0
#define __UINT8_MAX__ 255
#define __ACCUM_FBIT__ 15
#define __ANDROID__ 1
#define __WINT_MAX__ 4294967295U
#define __USFRACT_FBIT__ 8
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __SIZE_MAX__ 4294967295U
#define __ARM_ARCH_ISA_ARM 1
#define __WCHAR_MAX__ 4294967295U
#define __LACCUM_IBIT__ 32
#define __DBL_DENORM_MIN__ ((double)4.9406564584124654e-324L)
#define __GCC_ATOMIC_CHAR_LOCK_FREE 1
#define __FLT_EVAL_METHOD__ 0
#define __unix__ 1
#define __LLACCUM_MAX__ 0X7FFFFFFFFFFFFFFFP-31LLK
#define __GCC_ATOMIC_CHAR32_T_LOCK_FREE 1
#define __FRACT_FBIT__ 15
#define __UINT_FAST64_MAX__ 18446744073709551615ULL
#define __SIG_ATOMIC_TYPE__ int
#define __UACCUM_FBIT__ 16
#define __DBL_MIN_10_EXP__ (-307)
#define __FINITE_MATH_ONLY__ 0
#define __ARMEL__ 1
#define __LFRACT_IBIT__ 0
#define __GNUC_PATCHLEVEL__ 0
#define __LFRACT_MAX__ 0X7FFFFFFFP-31LR
#define __UINT_FAST8_MAX__ 255
#define __DEC64_MAX_EXP__ 385
#define __INT8_C(c) c
#define __UINT_LEAST64_MAX__ 18446744073709551615ULL
#define __SA_FBIT__ 15
#define __SHRT_MAX__ 32767
#define __LDBL_MAX__ 1.7976931348623157e+308L
#define __FRACT_MAX__ 0X7FFFP-15R
#define __ARM_ARCH_5TE__ 1
#define __UFRACT_FBIT__ 16
#define __ARM_FP 12
#define __UFRACT_MIN__ 0.0UR
#define __UINT_LEAST8_MAX__ 255
#define __GCC_ATOMIC_BOOL_LOCK_FREE 1
#define __UINTMAX_TYPE__ long long unsigned int
#define __LLFRACT_EPSILON__ 0x1P-63LLR
#define __linux 1
#define __DEC32_EPSILON__ 1E-6DF
#define __CHAR_UNSIGNED__ 1
#define __UINT32_MAX__ 4294967295U
#define __ULFRACT_MAX__ 0XFFFFFFFFP-32ULR
#define __TA_IBIT__ 64
#define __LDBL_MAX_EXP__ 1024
#define __WINT_MIN__ 0U
#define __linux__ 1
#define __ULLFRACT_MIN__ 0.0ULLR
#define __SCHAR_MAX__ 127
#define __WCHAR_MIN__ 0U
#define __INT64_C(c) c ## LL
#define __DBL_DIG__ 15
#define __ARM_NEON_FP 4
#define __GCC_ATOMIC_POINTER_LOCK_FREE 1
#define __LLACCUM_MIN__ (-0X1P31LLK-0X1P31LLK)
#define __SIZEOF_INT__ 4
#define __SIZEOF_POINTER__ 4
#define __USACCUM_IBIT__ 8
#define __USER_LABEL_PREFIX__
#define __STDC_HOSTED__ 1
#define __LDBL_HAS_INFINITY__ 1
#define __LFRACT_MIN__ (-0.5LR-0.5LR)
#define __HA_IBIT__ 8
#define __TQ_IBIT__ 0
#define __FLT_EPSILON__ 1.1920928955078125e-7F
#define __APCS_32__ 1
#define __USFRACT_IBIT__ 0
#define __LDBL_MIN__ 2.2250738585072014e-308L
#define __FRACT_MIN__ (-0.5R-0.5R)
#define __DEC32_MAX__ 9.999999E96DF
#define __DA_IBIT__ 32
#define __ARM_SIZEOF_MINIMAL_ENUM 4
#define __INT32_MAX__ 2147483647
#define __UQQ_FBIT__ 8
#define __SIZEOF_LONG__ 4
#define __UACCUM_MAX__ 0XFFFFFFFFP-16UK
#define __UINT16_C(c) c
#define __DECIMAL_DIG__ 17
#define __LFRACT_EPSILON__ 0x1P-31LR
#define __ULFRACT_MIN__ 0.0ULR
#define __LDBL_HAS_QUIET_NAN__ 1
#define __ULACCUM_IBIT__ 32
#define __UACCUM_EPSILON__ 0x1P-16UK
#define __GNUC__ 4
#define __ULLACCUM_MAX__ 0XFFFFFFFFFFFFFFFFP-32ULLK
#define __HQ_IBIT__ 0
#define __FLT_HAS_DENORM__ 1
#define __SIZEOF_LONG_DOUBLE__ 8
#define __BIGGEST_ALIGNMENT__ 8
#define __DQ_IBIT__ 0
#define __DBL_MAX__ ((double)1.7976931348623157e+308L)
#define __ULFRACT_IBIT__ 0
#define __INT_FAST32_MAX__ 2147483647
#define __DBL_HAS_INFINITY__ 1
#define __ACCUM_IBIT__ 16
#define __DEC32_MIN_EXP__ (-94)
#define __THUMB_INTERWORK__ 1
#define __LACCUM_MAX__ 0X7FFFFFFFFFFFFFFFP-31LK
#define __INT_FAST16_TYPE__ int
#define __LDBL_HAS_DENORM__ 1
#define __DEC128_MAX__ 9.999999999999999999999999999999999E6144DL
#define __INT_LEAST32_MAX__ 2147483647
#define __ARM_PCS 1
#define __DEC32_MIN__ 1E-95DF
#define __ACCUM_MAX__ 0X7FFFFFFFP-15K
#define __DBL_MAX_EXP__ 1024
#define __USACCUM_EPSILON__ 0x1P-8UHK
#define __DEC128_EPSILON__ 1E-33DL
#define __SFRACT_MAX__ 0X7FP-7HR
#define __FRACT_IBIT__ 0
#define __PTRDIFF_MAX__ 2147483647
#define __UACCUM_MIN__ 0.0UK
#define __UACCUM_IBIT__ 16
#define __LONG_LONG_MAX__ 9223372036854775807LL
#define __SIZEOF_SIZE_T__ 4
#define __ULACCUM_MAX__ 0XFFFFFFFFFFFFFFFFP-32ULK
#define __SIZEOF_WINT_T__ 4
#define __SA_IBIT__ 16
#define __ULLACCUM_MIN__ 0.0ULLK
#define __GXX_ABI_VERSION 1002
#define __UTA_FBIT__ 64
#define __SOFTFP__ 1
#define __FLT_MIN_EXP__ (-125)
#define __USFRACT_MAX__ 0XFFP-8UHR
#define __UFRACT_IBIT__ 0
#define __ARM_FEATURE_QBIT 1
#define __INT_FAST64_TYPE__ long long int
#define __DBL_MIN__ ((double)2.2250738585072014e-308L)
#define __LACCUM_MIN__ (-0X1P31LK-0X1P31LK)
#define __ULLACCUM_FBIT__ 32
#define __GXX_TYPEINFO_EQUALITY_INLINE 0
#define __ULLFRACT_EPSILON__ 0x1P-64ULLR
#define __DEC128_MIN__ 1E-6143DL
#define __REGISTER_PREFIX__
#define __UINT16_MAX__ 65535
#define __DBL_HAS_DENORM__ 1
#define __ACCUM_MIN__ (-0X1P15K-0X1P15K)
#define __SQ_IBIT__ 0
#define __UINT8_TYPE__ unsigned char
#define __UHA_FBIT__ 8
#define __NO_INLINE__ 1
#define __SFRACT_MIN__ (-0.5HR-0.5HR)
#define __UTQ_FBIT__ 128
#define __FLT_MANT_DIG__ 24
#define __VERSION__ "4.8"
#define __UINT64_C(c) c ## ULL
#define __ULLFRACT_FBIT__ 64
#define __FRACT_EPSILON__ 0x1P-15R
#define __ULACCUM_MIN__ 0.0ULK
#define __UDA_FBIT__ 32
#define __LLACCUM_EPSILON__ 0x1P-31LLK
#define __GCC_ATOMIC_INT_LOCK_FREE 1
#define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __USFRACT_MIN__ 0.0UHR
#define __UQQ_IBIT__ 0
#define __INT32_C(c) c
#define __DEC64_EPSILON__ 1E-15DD
#define __ORDER_PDP_ENDIAN__ 3412
#define __DEC128_MIN_EXP__ (-6142)
#define __UHQ_FBIT__ 16
#define __LLACCUM_FBIT__ 31
#define __INT_FAST32_TYPE__ int
#define __UINT_LEAST16_TYPE__ short unsigned int
#define unix 1
#define __INT16_MAX__ 32767
#define __SIZE_TYPE__ unsigned int
#define __UINT64_MAX__ 18446744073709551615ULL
#define __UDQ_FBIT__ 64
#define __INT8_TYPE__ signed char
#define __ELF__ 1
#define __ULFRACT_EPSILON__ 0x1P-32ULR
#define __LLFRACT_FBIT__ 63
#define __FLT_RADIX__ 2
#define __INT_LEAST16_TYPE__ short int
#define __LDBL_EPSILON__ 2.2204460492503131e-16L
#define __UINTMAX_C(c) c ## ULL
#define __SACCUM_MAX__ 0X7FFFP-7HK
#define __SIG_ATOMIC_MAX__ 2147483647
#define __GCC_ATOMIC_WCHAR_T_LOCK_FREE 1
#define __VFP_FP__ 1
#define __SIZEOF_PTRDIFF_T__ 4
#define __LACCUM_EPSILON__ 0x1P-31LK
#define __DEC32_SUBNORMAL_MIN__ 0.000001E-95DF
#define __INT_FAST16_MAX__ 2147483647
#define __UINT_FAST32_MAX__ 4294967295U
#define __UINT_LEAST64_TYPE__ long long unsigned int
#define __USACCUM_MAX__ 0XFFFFP-8UHK
#define __SFRACT_EPSILON__ 0x1P-7HR
#define __FLT_HAS_QUIET_NAN__ 1
#define __FLT_MAX_10_EXP__ 38
#define __LONG_MAX__ 2147483647L
#define __DEC128_SUBNORMAL_MIN__ 0.000000000000000000000000000000001E-6143DL
#define __FLT_HAS_INFINITY__ 1
#define __unix 1
#define __USA_FBIT__ 16
#define __UINT_FAST16_TYPE__ unsigned int
#define __DEC64_MAX__ 9.999999999999999E384DD
#define __CHAR16_TYPE__ short unsigned int
#define __PRAGMA_REDEFINE_EXTNAME 1
#define __INT_LEAST16_MAX__ 32767
#define __DEC64_MANT_DIG__ 16
#define __INT64_MAX__ 9223372036854775807LL
#define __UINT_LEAST32_MAX__ 4294967295U
#define __SACCUM_FBIT__ 7
#define __GCC_ATOMIC_LONG_LOCK_FREE 1
#define __INT_LEAST64_TYPE__ long long int
#define __ARM_FEATURE_CLZ 1
#define __INT16_TYPE__ short int
#define __INT_LEAST8_TYPE__ signed char
#define __SQ_FBIT__ 31
#define __DEC32_MAX_EXP__ 97
#define __ARM_ARCH_ISA_THUMB 1
#define __INT_FAST8_MAX__ 127
#define __ARM_ARCH 5
#define __INTPTR_MAX__ 2147483647
#define __QQ_FBIT__ 7
#define linux 1
#define __UTA_IBIT__ 64
#define __LDBL_MANT_DIG__ 53
#define __SFRACT_FBIT__ 7
#define __SACCUM_MIN__ (-0X1P7HK-0X1P7HK)
#define __DBL_HAS_QUIET_NAN__ 1
#define __SIG_ATOMIC_MIN__ (-__SIG_ATOMIC_MAX__ - 1)
#define __INTPTR_TYPE__ int
#define __UINT16_TYPE__ short unsigned int
#define __WCHAR_TYPE__ unsigned int
#define __SIZEOF_FLOAT__ 4
#define __USQ_FBIT__ 32
#define __pic__ 1
#define __UINTPTR_MAX__ 4294967295U
#define __DEC64_MIN_EXP__ (-382)
#define __ULLACCUM_IBIT__ 32
#define __INT_FAST64_MAX__ 9223372036854775807LL
#define __GCC_ATOMIC_TEST_AND_SET_TRUEVAL 1
#define __FLT_DIG__ 6
#define __UINT_FAST64_TYPE__ long long unsigned int
#define __INT_MAX__ 2147483647
#define __LACCUM_FBIT__ 31
#define __USACCUM_MIN__ 0.0UHK
#define __UHA_IBIT__ 8
#define __INT64_TYPE__ long long int
#define __FLT_MAX_EXP__ 128
#define __UTQ_IBIT__ 0
#define __DBL_MANT_DIG__ 53
#define __INT_LEAST64_MAX__ 9223372036854775807LL
#define __GCC_ATOMIC_CHAR16_T_LOCK_FREE 1
#define __DEC64_MIN__ 1E-383DD
#define __WINT_TYPE__ unsigned int
#define __UINT_LEAST32_TYPE__ unsigned int
#define __SIZEOF_SHORT__ 2
#define __ULLFRACT_IBIT__ 0
#define __LDBL_MIN_EXP__ (-1021)
#define __arm__ 1
#define __UDA_IBIT__ 32
#define __INT_LEAST8_MAX__ 127
#define __LFRACT_FBIT__ 31
#define __LDBL_MAX_10_EXP__ 308
#define __ATOMIC_RELAXED 0
#define __DBL_EPSILON__ ((double)2.2204460492503131e-16L)
#define __UINT8_C(c) c
#define __INT_LEAST32_TYPE__ int
#define __SIZEOF_WCHAR_T__ 4
#define __UINT64_TYPE__ long long unsigned int
#define __LLFRACT_MAX__ 0X7FFFFFFFFFFFFFFFP-63LLR
#define __TQ_FBIT__ 127
#define __INT_FAST8_TYPE__ signed char
#define __ULLACCUM_EPSILON__ 0x1P-32ULLK
#define __UHQ_IBIT__ 0
#define __LLACCUM_IBIT__ 32
#define __DBL_DECIMAL_DIG__ 17
#define __DEC_EVAL_METHOD__ 2
#define __TA_FBIT__ 63
#define __UDQ_IBIT__ 0
#define __ORDER_BIG_ENDIAN__ 4321
#define __ACCUM_EPSILON__ 0x1P-15K
#define __UINT32_C(c) c ## U
#define __INTMAX_MAX__ 9223372036854775807LL
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __FLT_DENORM_MIN__ 1.4012984643248171e-45F
#define __LLFRACT_IBIT__ 0
#define __INT8_MAX__ 127
#define __PIC__ 1
#define __UINT_FAST32_TYPE__ unsigned int
#define __CHAR32_TYPE__ unsigned int
#define __FLT_MAX__ 3.4028234663852886e+38F
#define __USACCUM_FBIT__ 8
#define __INT32_TYPE__ int
#define __SIZEOF_DOUBLE__ 8
#define __FLT_MIN_10_EXP__ (-37)
#define __UFRACT_EPSILON__ 0x1P-16UR
#define __INTMAX_TYPE__ long long int
#define __DEC128_MAX_EXP__ 6145
#define __ATOMIC_CONSUME 1
#define __GNUC_MINOR__ 8
#define __UINTMAX_MAX__ 18446744073709551615ULL
#define __DEC32_MANT_DIG__ 7
#define __HA_FBIT__ 7
#define __DBL_MAX_10_EXP__ 308
#define __LDBL_DENORM_MIN__ 4.9406564584124654e-324L
#define __INT16_C(c) c
#define __STDC__ 1
#define __PTRDIFF_TYPE__ int
#define __LLFRACT_MIN__ (-0.5LLR-0.5LLR)
#define __ATOMIC_SEQ_CST 5
#define __DA_FBIT__ 31
#define __UINT32_TYPE__ unsigned int
#define __UINTPTR_TYPE__ unsigned int
#define __USA_IBIT__ 16
#define __DEC64_SUBNORMAL_MIN__ 0.000000000000001E-383DD
#define __ARM_EABI__ 1
#define __DEC128_MANT_DIG__ 34
#define __LDBL_MIN_10_EXP__ (-307)
#define __SIZEOF_LONG_LONG__ 8
#define __ULACCUM_EPSILON__ 0x1P-32ULK
#define __SACCUM_IBIT__ 8
#define __GCC_ATOMIC_LLONG_LOCK_FREE 1
#define __LDBL_DIG__ 15
#define __FLT_DECIMAL_DIG__ 9
#define __UINT_FAST16_MAX__ 4294967295U
#define __GNUC_GNU_INLINE__ 1
#define __GCC_ATOMIC_SHORT_LOCK_FREE 1
#define __ULLFRACT_MAX__ 0XFFFFFFFFFFFFFFFFP-64ULLR
#define __UINT_FAST8_TYPE__ unsigned char
#define __USFRACT_EPSILON__ 0x1P-8UHR
#define __ULACCUM_FBIT__ 32
#define __ARM_FEATURE_DSP 1
#define __QQ_IBIT__ 0
#define __ATOMIC_ACQ_REL 4
#define __ATOMIC_RELEASE 3
Title: Re:
Post by: MortenMacFly on August 30, 2014, 10:39:46 pm
Well did you enable macro expansion in the opTions?
Title: Re:
Post by: MortenMacFly on August 30, 2014, 10:43:27 pm
...additionally: how did you define the compiler? A copy of gcc? Can you provide the compiler section of your config file and/or the xml compiler settings?
Title: Re:
Post by: SteelRat on August 30, 2014, 11:32:19 pm
Well did you enable macro expansion in the opTions?
If there no some hidden options so - yes, all is enabled.
Compiler's XML based on ARM Compiler.
Code: [Select]
<?xml version="1.0"?>
<!DOCTYPE CodeBlocks_compiler>
<CodeBlocks_compiler name="GNU GCC Compiler for Android"
                     id="android-gcc"
                     weight="60">
    <Path type="master">
        <Search envVar="PATH"
                for="C"/>
        <if platform="windows">
            <Search path="C:\ndk*"/>
            <Fallback path="C:\ndk"/>
        </if>
        <else>
            <Fallback path="$NDK"/>
        </else>
    </Path>
    <if platform="windows">
        <Path type="include">
            <Add><master/>/toolchains/arm-linux-androideabi-4.8/prebuilt/linux-x86_64/include</Add>
        </Path>
        <Path type="lib">
            <Add><master/>/toolchains/arm-linux-androideabi-4.8/prebuilt/linux-x86_64/lib</Add>
        </Path>
        <Path type="extra">
            <Add><master/>/tools</Add>
        </Path>
    </if>
    <else>
        <Path type="include">
            <Add><master/>/toolchains/arm-linux-androideabi-4.8/prebuilt/linux-x86_64/include</Add>
        </Path>
        <Path type="lib">
            <Add><master/>/toolchains/arm-linux-androideabi-4.8/prebuilt/linux-x86_64/lib</Add>
        </Path>
    </else>
</CodeBlocks_compiler>

Code: [Select]
<?xml version="1.0"?>
<!DOCTYPE CodeBlocks_compiler_options>
<CodeBlocks_compiler_options>
    <if platform="windows">
        <Program name="C"         value="arm-linux-androideabi-gcc.exe"/>
        <Program name="CPP"       value="arm-linux-androideabi-g++.exe"/>
        <Program name="LD"        value="arm-linux-androideabi-g++.exe"/>
        <Program name="DBGconfig" value=""/>
        <Program name="LIB"       value="arm-linux-androideabi-ar.exe"/>
        <Program name="WINDRES"   value=""/>
        <Program name="MAKE"      value="make.exe"/>
    </if>
    <else>
        <Program name="C"         value="arm-linux-androideabi-gcc"/>
        <Program name="CPP"       value="arm-linux-androideabi-g++"/>
        <Program name="LD"        value="arm-linux-androideabi-g++"/>
        <Program name="DBGconfig" value=""/>
        <Program name="LIB"       value="arm-linux-androideabi-ar"/>
        <Program name="WINDRES"   value=""/>
        <Program name="MAKE"      value="ndk-build"/>
    </else>

    <Switch name="includeDirs"             value="-I"/>
    <Switch name="libDirs"                 value="-L"/>
    <Switch name="linkLibs"                value="-l"/>
    <Switch name="defines"                 value="-D"/>
    <Switch name="genericSwitch"           value="-"/>
    <Switch name="objectExtension"         value="o"/>
    <Switch name="needDependencies"        value="true"/>
    <Switch name="forceCompilerUseQuotes"  value="false"/>
    <Switch name="forceLinkerUseQuotes"    value="false"/>
    <Switch name="logging"                 value="default"/>
    <Switch name="libPrefix"               value="lib"/>
    <Switch name="libExtension"            value="a"/>
    <Switch name="linkerNeedsLibPrefix"    value="false"/>
    <Switch name="linkerNeedsLibExtension" value="false"/>

    <Option name="Produce debugging symbols"
            option="-g"
            category="Debugging"
            checkAgainst="-O -O1 -O2 -O3 -Os"
            checkMessage="You have optimizations enabled. This is Not A Good Thing(tm) when producing debugging symbols..."
            supersedes="-s"/>

    <if platform="windows">
        <Option name="Profile code when executed"
                option="-pg"
                category="Profiling"
                additionalLibs="-pg -lgmon"
                supersedes="-s"/>
    </if>
    <else>
        <Option name="Profile code when executed"
                option="-pg"
                category="Profiling"
                additionalLibs="-pg"
                supersedes="-s"/>
    </else>

    <Common name="warnings"/>
    <Category name="Warnings">
        <Option name="Enable Effective-C++ warnings (thanks Scott Meyers)"
                option="-Weffc++"/>
        <Option name="Warn whenever a switch statement does not have a default case"
                option="-Wswitch-default"/>
        <Option name="Warn whenever a switch statement has an index of enumerated type and lacks a case for one or more of the named codes of that enumeration"
                option="-Wswitch-enum"/>
        <Option name="Warn if a user supplied include directory does not exist"
                option="-Wmissing-include-dirs"/>
        <Option name="Warn if a global function is defined without a previous declaration"
                option="-Wmissing-declarations"/>
        <Option name="Warn if the compiler detects that code will never be executed"
                option="-Wunreachable-code"/>
        <Option name="Warn if a function can not be inlined and it was declared as inline"
                option="-Winline"/>
        <Option name="Warn if floating point values are used in equality comparisons"
                option="-Wfloat-equal"/>
        <Option name="Warn if an undefined identifier is evaluated in an '#if' directive"
                option="-Wundef"/>
        <Option name="Warn whenever a pointer is cast such that the required alignment of the target is increased"
                option="-Wcast-align"/>
        <Option name="Warn if anything is declared more than once in the same scope"
                option="-Wredundant-decls"/>
        <Option name="Warn about unitialized variables which are initialized with themselves"
                option="-Winit-self"/>
        <Option name="Warn whenever a local variable shadows another local variable, parameter or global variable or whenever a built-in function is shadowed"
                option="-Wshadow"/>
        <Option name="Warn if a class has virtual functions but no virtual destructor"
                option="-Wnon-virtual-dtor"/>
        <Option name="Check calls to printf and scanf, etc., to make sure that the arguments supplied have types appropriate to the format string specified, and that the conversions specified in the format string make sense, included in -Wall."
                option="-Wformat"/>
        <Option name="If -Wformat is specified, also warn about uses of format functions that represent possible security problems."
                option="-Werror=format-security"/>
    </Category>

    <Common name="optimization"/>
    <Category name="Optimization">
        <Option name="-fno-omit-frame-pointer"
              option="-fno-omit-frame-pointer"
              supersedes="-fomit-frame-pointer"/>

        <Option name="Don't keep the frame pointer in a register for functions that don't need one"
            option="-fomit-frame-pointer"
            checkAgainst="-g -ggdb"
            checkMessage="You have debugging symbols enabled. This is Not A Good Thing(tm) when optimizing..."/>
    </Category>

    <Category name="ARM CPU architecture specific">
        <Option name="Generate stack frame even if unnecessary"
                option="-mapcs-frame"
                supersedes="-mno-apcs-frame"/>
        <Option name="-mno-apcs-frame"
                option="-mno-apcs-frame"
                supersedes="-mapcs-frame"/>
        <Option name="-mabi=apcs-gnu"
                option="-mabi=apcs-gnu"
                supersedes="-mabi=atpcs -mabi=aapcs -mabi=aapcs-linux -mabi=iwmmxt"/>
        <Option name="-mabi=atpcs"
                option="-mabi=atpcs"
                supersedes="-mabi=apcs-gnu -mabi=aapcs -mabi=aapcs-linux -mabi=iwmmxt"/>
        <Option name="-mabi=aapcs"
                option="-mabi=aapcs"
                supersedes="-mabi=apcs-gnu -mabi=atpcs -mabi=aapcs-linux -mabi=iwmmxt"/>
        <Option name="-mabi=aapcs-linux"
                option="-mabi=aapcs-linux"
                supersedes="-mabi=apcs-gnu -mabi=atpcs -mabi=aapcs -mabi=iwmmxt"/>
        <Option name="-mabi=iwmmxt"
                option="-mabi=iwmmxt"
                supersedes="-mabi=apcs-gnu -mabi=atpcs -mabi=aapcs -mabi=aapcs-linux"/>
        <Option name="-mapcs-stack-check"
                option="-mapcs-stack-check"
                supersedes="-mno-apcs-stack-check"/>
        <Option name="-mno-apcs-stack-check"
                option="-mno-apcs-stack-check"
                supersedes="-mapcs-stack-check"/>
        <Option name="-mapcs-float"
                option="-mapcs-float"
                supersedes="-mno-apcs-float"/>
        <Option name="-mno-apcs-float"
                option="-mno-apcs-float"
                supersedes="-mapcs-float"/>
        <Option name="-mapcs-reentrant"
                option="-mapcs-reentrant"
                supersedes="-mno-apcs-reentrant"/>
        <Option name="-mno-apcs-reentrant"
                option="-mno-apcs-reentrant"
                supersedes="-mapcs-reentrant"/>
        <Option name="-msched-prolog"
                option="-msched-prolog"
                supersedes="-mno-sched-prolog"/>
        <Option name="Prevent the reordering of instructions in the function prolog"
                option="-mno-sched-prolog"
                supersedes="-msched-prolog"/>
        <Option name="Generate code for little-endian processors"
                option="-mlittle-endian"
                supersedes="-mbig-endian"/>
        <Option name="Generate code for big-endian processors"
                option="-mbig-endian"
                supersedes="-mlittle-endian"/>
        <Option name="-mwords-little-endian"
                option="-mwords-little-endian"/>
        <Option name="-msoft-float"
                option="-msoft-float"
                supersedes="-mhard-float"/>
        <Option name="-mhard-float"
                option="-mhard-float"
                supersedes="-msoft-float"/>
        <Option name="-mfloat-abi=softfp"
                option="-mfloat-abi=softfp"
                supersedes="-msoft-float -mhard-float"/>
        <Option name="-mfpe"
                option="-mfpe"/>
        <Option name="Generate code supporting calls between the ARM and Thumb instruction sets"
                option="-mthumb-interwork"
                supersedes="-mno-thumb-interwork -marm -mthumb"/>
        <Option name="-mno-thumb-interwork"
                option="-mno-thumb-interwork"
                supersedes="-mthumb-interwork"/>
        <Option name="'fpa' floating point unit"
                option="-mfpu=fpa"
                supersedes="-mfpu=fpe2 -mfpu=fpe3 -mfpu=maverick -mfpu=vfp -mfpu=vfpv3 -mfpu=vfpv3-d16 -mfpu=neon"/>
        <Option name="'fpe2' floating point unit"
                option="-mfpu=fpe2"
                supersedes="-mfpu=fpa -mfpu=fpe3 -mfpu=maverick -mfpu=vfp -mfpu=vfpv3 -mfpu=vfpv3-d16 -mfpu=neon"/>
        <Option name="'fpe3' floating point unit"
                option="-mfpu=fpe3"
                supersedes="-mfpu=fpa -mfpu=fpe2 -mfpu=maverick -mfpu=vfp -mfpu=vfpv3 -mfpu=vfpv3-d16 -mfpu=neon"/>
        <Option name="'maverick' floating point unit"
                option="-mfpu=maverick"
                supersedes="-mfpu=fpa -mfpu=fpe2 -mfpu=fpe3 -mfpu=vfp -mfpu=vfpv3 -mfpu=vfpv3-d16 -mfpu=neon"/>
        <Option name="'vfp' floating point unit"
                option="-mfpu=vfp"
                supersedes="-mfpu=fpa -mfpu=fpe2 -mfpu=fpe3 -mfpu=maverick -mfpu=vfpv3 -mfpu=vfpv3-d16 -mfpu=neon"/>
        <Option name="'vfpv3' floating point unit"
                option="-mfpu=vfpv3"
                supersedes="-mfpu=fpa -mfpu=fpe2 -mfpu=fpe3 -mfpu=maverick -mfpu=vfp -mfpu=vfpv3-d16 -mfpu=neon"/>
        <Option name="'vfpv3-d16' floating point unit"
                option="-mfpu=vfpv3-d16"
                supersedes="-mfpu=fpa -mfpu=fpe2 -mfpu=fpe3 -mfpu=maverick -mfpu=vfp -mfpu=vfpv3 -mfpu=neon"/>
        <Option name="'neon' floating point unit"
                option="-mfpu=neon"
                supersedes="-mfpu=fpa -mfpu=fpe2 -mfpu=fpe3 -mfpu=maverick -mfpu=vfp -mfpu=vfpv3 -mfpu=vfpv3-d16"/>
        <Option name="Round structure and union size up to a multiple of 8 bits"
                option="-mstructure-size-boundary=8"
                supersedes="-mstructure-size-boundary=32 -mstructure-size-boundary=64"/>
        <Option name="Round structure and union size up to a multiple of 32 bits"
                option="-mstructure-size-boundary=32"
                supersedes="-mstructure-size-boundary=8 -mstructure-size-boundary=64"/>
        <Option name="Round structure and union size up to a multiple of 64 bits"
                option="-mstructure-size-boundary=64"
                supersedes="-mstructure-size-boundary=8 -mstructure-size-boundary=32"/>
        <Option name="Generate a call to the function abort at the end of a noreturn function"
                option="-mabort-on-noreturn"/>
        <Option name="-mlong-calls"
                option="-mlong-calls"
                supersedes="-mno-long-calls"/>
        <Option name="-mno-long-calls"
                option="-mno-long-calls"
                supersedes="-mlong-calls"/>
        <Option name="-mnop-fun-dllimport"
                option="-mnop-fun-dllimport"/>
        <Option name="-mpoke-function-name"
                option="-mpoke-function-name"/>
        <Option name="Generate code that executes in Thumb state"
                option="-mthumb"
                supersedes="-marm -mthumb-interwork"/>
        <Option name="Generate code that executes in ARM state"
                option="-marm"
                supersedes="-mthumb -mthumb-interwork"/>
     </Category>

    <Category name="Other">
        <Option name="-fpic"
                option="-fpic"/>
        <Option name="-ffunction-sections"
                option="-ffunction-sections"/>
        <Option name="-funwind-tables"
                option="-funwind-tables"/>
        <Option name="-fstack-protector"
                option="-fstack-protector"/>
        <Option name="-no-canonical-prefixes"
                option="-no-canonical-prefixes"/>
        <Option name="-fno-exceptions"
                option="-fno-exceptions"/>
        <Option name="-fno-rtti"
                option="-fno-rtti"/>
        <Option name="No exec stack"
              option="-Wa,--noexecstack"/>
        <Option name="Like -MD except mention only user header files, not system header files."
              option="-MMD"/>
        <Option name="This option instructs CPP to add a phony target for each dependency other than the main file, causing each to depend on nothing."
              option="-MP"/>
        <Option name="-fno-strict-aliasing"
              option="-fno-strict-aliasing"/>
    </Category>

    <Category name="ARM CPU architecture derivatives"
              exclusive="true">
        <Option name="Armeabi-v7a"
                option="-march=armv7-a"/>
    </Category>

    <Command name="CompileObject"
             value="$compiler $options $includes -c $file -o $object"/>
    <Command name="GenDependencies"
             value="$compiler -MM $options -MF $dep_object -MT $object $includes $file"/>
    <Command name="CompileResource"
             value="$rescomp -i $file -J rc -o $resource_output -O coff $res_includes"/>
    <Command name="LinkConsoleExe"
             value="$linker $libdirs -o $exe_output $link_objects $link_resobjects $link_options $libs"/>
    <if platform="windows">
        <Command name="LinkExe"
                 value="$linker $libdirs -o $exe_output $link_objects $link_resobjects $link_options $libs -mwindows"/>
        <Command name="LinkDynamic"
                 value="$linker -shared -Wl,--output-def=$def_output -Wl,--out-implib=$static_output -Wl,--dll $libdirs $link_objects $link_resobjects -o $exe_output $link_options $libs"/>
    </if>
    <else>
        <Command name="LinkExe"
                 value="$linker $libdirs -o $exe_output $link_objects $link_resobjects $link_options $libs"/>
        <Command name="LinkDynamic"
                 value="$linker -shared $libdirs $link_objects $link_resobjects -o $exe_output $link_options $libs"/>
    </else>
    <Command name="LinkNative"
             value="$linker $libdirs -o $exe_output $link_objects $link_resobjects $link_options $libs"/>
    <Command name="LinkStatic"
             value="$lib_linker -r -s $static_output $link_objects"/>
    <Common name="cmds"/>

    <Common name="re"/>

    <Common name="sort"/>
</CodeBlocks_compiler_options>
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 02, 2014, 07:51:54 am
Here is a patch that adds support for:
1) Nested unnamed (struct or union) within unnamed: all the members should be invokable from the parent class or struct.
2) Show tooltips for members of unnamed / enum within class invoked directly (also for nested cases).


Hi, Huki, thanks for the contribution, this patch is in trunk now(rev 9901) with some comments added by me.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 02, 2014, 09:35:17 am
Hi, with the cctest project, I can get such beautiful result:

Code: [Select]
-PASS: ClassA::  a
-PASS: ClassA::  s
-PASS: ClassA::  u
-PASS: ClassA::  v
-PASS: ClassA::  STRUCT_A
-PASS: ClassA::  STRUCT_B
-PASS: ClassA::  UNNAMED_A
-PASS: ClassA::  UNNAMED_B
-PASS: ClassA::  ENUM_A
-PASS: ClassA::  ENUM_B
--------------------------------------------------------
Total 10 tests, 10 PASS, 0 FAIL
--------------------------------------------------------

Here is the test file:
Code: [Select]
class ClassA
{
public:
    int a;
    struct A
    {
        int c;
    } z;

    // unnamed struct
    struct
    {
        int s;

        union
        {
            int u;
            short v;
        };

        enum EnumS
        {
            STRUCT_A,
            STRUCT_B,
        };
    };

    enum
    {
        UNNAMED_A,
        UNNAMED_B
    };

    enum EnumA
    {
        ENUM_A,
        ENUM_B
    };
};

// Hovering on following member variables should
// show tooltip: a, s, u, v, all the enums like STRUCT_A, etc.
// not c

//ClassA:: //a,s,u,v,STRUCT_A,STRUCT_B,UNNAMED_A,UNNAMED_B,ENUM_A,ENUM_B

Title: Re: Several improvements to Code Completion plugin
Post by: SteelRat on September 04, 2014, 11:01:38 am
I still don't understand logic of inactive code's highlighting or is it bug?
Little test for example:

Code: Text
  1. #ifndef _MAIN_H_
  2. #define _MAIN_H_
  3.  
  4. #define TEST 0
  5.  
  6. #endif
  7.  

Code: C++
  1. #include "main.h"
  2.  
  3. //#define TEST 0
  4.  
  5. int main()
  6. {
  7.  
  8. #if defined(TEST)
  9.         int = 0;
  10. #else
  11.         int = 1;
  12. #end
  13.  
  14. #ifdef TEST
  15.         int n = 0;
  16. #else
  17.         int n = 1;
  18. #endif
  19.  
  20. #if TEST == 1
  21.         int n = 2;
  22. #endif
  23.  
  24. #if TEST == 0
  25.         int n = 3;
  26. #endif
  27. }

If TEST defined in header - it is ignored always, it just does not exist for highlighting. CC can't find declaration until opening Symbols browser - then it can. If TEST defined in the same file it has strange behaviour with defined: TEST, TEST 1, TEST 2. For example, "#if defined" works well, but next "#if TEST == 1" - ignored.

Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 04, 2014, 04:04:42 pm
CC can't find declaration until opening Symbols browser - then it can.
I can't reproduce this bug, it works fine here with your test code.

BTW: CC has nothing to do with active C preprocessor highlighting, highlighting was done purely by scintilla editor control.

Title: Re: Several improvements to Code Completion plugin
Post by: SteelRat on September 04, 2014, 07:03:22 pm
Can i modify scintilla myself? I mean license permission. When there will be some time for it.
Title: Re: Several improvements to Code Completion plugin
Post by: Huki 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. :)
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg on September 17, 2014, 05:45:00 am
I'm thinking that the code:
Code: [Select]
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: [Select]
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: [Select]
* * cc_branch: applied patch with rewritten function-like macro handle (v5)
git-svn-id: http://svn.code.sf.net/p/codeblocks/code/branches/[email protected]  
It is used to expand function-like macro usage.

EDIT2, the second parameter of ReplaceFunctionLikeMacro() as added in
Code: [Select]
* * 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/[email protected]


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: [Select]
+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: [Select]
+        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: [Select]
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().

Title: Re: Several improvements to Code Completion plugin
Post by: Huki on September 18, 2014, 12:56:11 pm
I'm thinking that the code:
[...]
Should be changed to

Code: [Select]
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.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg 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).
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg 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. (http://forums.codeblocks.org/index.php/topic,19016.msg130216.html#msg130216)
I found that all the identifier like token should be checked for macro replacement, inside the DoGetToken(), other wise
Code: [Select]
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.
Title: Re: Several improvements to Code Completion plugin
Post by: ollydbg 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 (http://forums.codeblocks.org/index.php/topic,20175.msg137555.html#msg137555).
Any idea how to fix this?
Thanks.