Developer forums (C::B DEVELOPMENT STRICTLY!) > Development
Several improvements to Code Completion plugin
ollydbg:
--- Quote from: 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.
--- End quote ---
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: --- //FIX: Moved above.
/*if (peek==ParserConsts::colon)
{
// bit specifier (eg, xxx:1)
// -> walk to , or }
SkipToOneOfChars(ParserConsts::commaclbrace);
}*/
--- End code ---
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: --- if (peek==ParserConsts::colon)
{
peek = SkipToOneOfChars(ParserConsts::equals + ParserConsts::commaclbrace);
}
--- End code ---
About the include file expansion patch:
--- Quote from: Huki on February 02, 2014, 04:10:04 pm ---Hi, been busy for a while...
--- Quote from: ollydbg on January 28, 2014, 10:28:03 am ---
--- Quote from: 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, I guess the reason is:
If you recursively parse the #include directive.
--- Code: --- 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)
--- End code ---
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.
--- End quote ---
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.
--- End quote ---
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!
--- End quote ---
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.
ollydbg:
--- Quote from: ollydbg on February 08, 2014, 10:23:45 am ---
--- Quote from: 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.
--- End quote ---
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: --- //FIX: Moved above.
/*if (peek==ParserConsts::colon)
{
// bit specifier (eg, xxx:1)
// -> walk to , or }
SkipToOneOfChars(ParserConsts::commaclbrace);
}*/
--- End code ---
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: --- if (peek==ParserConsts::colon)
{
peek = SkipToOneOfChars(ParserConsts::equals + ParserConsts::commaclbrace);
}
--- End code ---
--- End quote ---
I think the answer is YES, so I commit the patch to trunk now rev 9642, thanks!
ollydbg:
@Huki, some regression was reported, see Re: The 08 February 2014 build (9639) is out., 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!
Huki:
--- Quote from: ollydbg on February 08, 2014, 10:23:45 am ---About the include file expansion patch:
--- Quote from: Huki on February 02, 2014, 04:10:04 pm ---Hi, been busy for a while...
--- Quote from: ollydbg on January 28, 2014, 10:28:03 am ---
--- Quote from: 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, I guess the reason is:
If you recursively parse the #include directive.
--- Code: --- 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)
--- End code ---
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.
--- End quote ---
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.
--- End quote ---
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!
--- End quote ---
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??
--- End quote ---
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. :)
ollydbg:
--- Quote from: Huki on February 15, 2014, 03:50:49 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).
--- End quote ---
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.
--- End quote ---
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. :)
--- End quote ---
Sure, me to, we can get more accurate tokens.
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version