Author Topic: Rewrote the DoParse function  (Read 11882 times)

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Rewrote the DoParse function
« on: April 19, 2010, 03:55:55 pm »
I rewrote the DoParse function, replace "else if" to "switch, case", found that performance has improved.
And now, the DoParse function like this:

Code
void ParserThread::DoParse()
{
    ...
        bool switchHandled = true;
        switch (token.Length())
        {
            case 1:
            if (token == ParserConsts::semicolon)
            {
                ...
            }
            else if (token == ParserConsts::dot
                     || (token == ParserConsts::gt && m_LastToken == ParserConsts::dash))
            {
                ...
            }
            else if (token == ParserConsts::opbrace)
            {
                ...
            }
            else if (token == ParserConsts::clbrace)
            {
                ...
            }
            else if (token == ParserConsts::colon)
            {
               ...
            }
            else if (token == ParserConsts::hash)
            {
                ...
            }
            else
                switchHandled = false;
            break;

            case 2:
            if (token == ParserConsts::kw_if || token == ParserConsts::kw_do)
            {
                ...
            }
            else
                switchHandled = false;
            break;

            case 3:
            if (token == ParserConsts::kw_for)
            {
                ...
            }
            else
                switchHandled = false;
            break;
......

            case 9:
            if (token == ParserConsts::kw_namespace)
            {
                m_Str.Clear();
                HandleNamespace();
            }
            else
                switchHandled = false;
            break;

            default:
                switchHandled = false;
            break;

[attachment deleted by admin]
« Last Edit: April 19, 2010, 03:59:29 pm by Loaden »

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9660
Re: Rewrote the DoParse function
« Reply #1 on: April 19, 2010, 06:28:06 pm »
Hi Loaden,
I fully respect all the work you are doing. Hence please keep in mind that we need to track all your ideas for a proper later integration. Unluckily it comes to a moment where we are in feature freeze. I am hardly trying to keep everything you are proposing but it's getting harder and harder with every patch of yours. So my request would be, can you please track everything in a way that we can have all patches in one place? Probably you can post all of them in an own thread of yours, just a short information:
- what do they do
- what do they modify
- what's the base SVN revision to apply them to
- (what other patches are required for the to apply)
- what are conflicts with other patches
- link to another place in the forums for discussions.
Inform that this thread is a collection of patches and that answers are not welcome (I can even remove discussion).
I realised that you already conflict with a patch of OllyDbg. It will be hard to resolve this in the end and I don't want to take such a risk later on.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline JGM

  • Lives here!
  • ****
  • Posts: 518
  • Got to practice :)
Re: Rewrote the DoParse function
« Reply #2 on: April 19, 2010, 06:52:42 pm »
I think the problem here is that english is hard for him, he needs a mediator, some one that speaks on his native language, discuss with him his work and organize the ideas here, just a suggestion  :)

Would be a shame loosing track of his work because of lack in organization (communication problems?)

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: Rewrote the DoParse function
« Reply #3 on: April 19, 2010, 07:07:15 pm »
Sorry, I am not a professional programmer, I'm just a physics teacher.
I can not understand some replies, but one thing. That is i fully agree: After CB official release, discuss whether to apply my patch.
I have not studied English for 10 years. :(

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9660
Re: Rewrote the DoParse function
« Reply #4 on: April 19, 2010, 09:08:14 pm »
Sorry, I am not a professional programmer, I'm just a physics teacher.
Interesting combination: Physics teacher advancing CC. ;-)

So to make things simple: Please start an own thread and post there your patch and a short explanation (including base SVN revision) and nothing else. Next time you have a patch, do the same in this very thread (basically "reply"). I will make this topic sticky and move every "replies" to another thread. Thus we have a clear history of your work in one place.

Probably it's already too late, but I would love to see this from the beginning of your work. Probably another CC dev can help?!
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5721
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Rewrote the DoParse function
« Reply #5 on: April 20, 2010, 01:48:27 am »
I think the problem here is that english is hard for him, he needs a mediator, some one that speaks on his native language, discuss with him his work and organize the ideas here, just a suggestion  :)

Would be a shame loosing track of his work because of lack in organization (communication problems?)

I would like to help with the translation between Chinese and English.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5721
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Rewrote the DoParse function
« Reply #6 on: April 20, 2010, 03:24:31 am »
About loaden's the patch:

I found there is some code in your patch:
Code
switch (token.Length())

Why do a switch statement depend on the string length??

For me, I would do something like this ( You can find this method in any books teaching about compiler design ):

The low level lexer( or Tokenizer ), the "Token" returned from the Tokenizer should have two fields:
1, token type: identifier, open brace, close brace....  and these types were categories by different enum value.
2, token value: for a "identifier" type token, the value is a wxString, just the "identifier" 's name, for open brace type token, they don't have a "token value" field.


In the parserthtread code, we can do a switch by the "token type".
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: Rewrote the DoParse function
« Reply #7 on: April 20, 2010, 06:58:01 am »
Hi Loaden,
I fully respect all the work you are doing. Hence please keep in mind that we need to track all your ideas for a proper later integration. Unluckily it comes to a moment where we are in feature freeze. I am hardly trying to keep everything you are proposing but it's getting harder and harder with every patch of yours. So my request would be, can you please track everything in a way that we can have all patches in one place? Probably you can post all of them in an own thread of yours, just a short information:
- what do they do
- what do they modify
- what's the base SVN revision to apply them to
- (what other patches are required for the to apply)
- what are conflicts with other patches
- link to another place in the forums for discussions.
Inform that this thread is a collection of patches and that answers are not welcome (I can even remove discussion).
I realised that you already conflict with a patch of OllyDbg. It will be hard to resolve this in the end and I don't want to take such a risk later on.
OK, I will do it in the next two days.
« Last Edit: April 20, 2010, 07:05:57 am by Loaden »

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: Rewrote the DoParse function
« Reply #8 on: April 20, 2010, 07:09:14 am »
I think the problem here is that english is hard for him, he needs a mediator, some one that speaks on his native language, discuss with him his work and organize the ideas here, just a suggestion  :)

Would be a shame loosing track of his work because of lack in organization (communication problems?)

I would like to help with the translation between Chinese and English.
Thanks lot!

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9660
Re: Rewrote the DoParse function
« Reply #9 on: April 20, 2010, 11:55:28 am »
And now, the DoParse function like this:
Ok, I had a look. Besides a clearer look, what I saw in the first place is a GoTo. This is unfortunately a "no-go" for us. Please don't use GoTos in C++. This is very bad style and there is no reason for doing this.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: Rewrote the DoParse function
« Reply #10 on: April 20, 2010, 03:57:25 pm »
This patch only as a try, here you can change to "return".

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5721
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Rewrote the DoParse function
« Reply #11 on: April 27, 2010, 07:21:48 am »
I realised that you already conflict with a patch of OllyDbg. It will be hard to resolve this in the end and I don't want to take such a risk later on.
@MortenMacFly

Yes, Loaden's patch about "handling conditional preprocessor" is totally different with "my patch of handling Preprocessor". We use different methods, let me give a explanation:

My patch released in the "ptest" project, see here: ParserTester for codecompletion plugin do the main work in the Tokenizer class. That is to say, when a Tokenizer return a wxString, it will never return a wxString stream like "#"  "ifdef", because all the handling of conditional preprocesser directives was done in the Tokenizer::DoGetToken().

The aim I implement this was that I want to solve some problems caused by these code:

Code
void function ( int i, #ifdef AAA
                     int j)
#else
)
#endif
{
XXXX
};


You see, when a #if is appeared in the function arguments parentheses, the parser may fail (because the current SVN code just match a closing parenthesis after it see a opening parenthesis) . But I'd say these kind of code happens in a rare situation. :D

Refer to Loaden's patch, he just do the "handling conditioanl preprocessor" in the Parserthread class. So, he didn't change Tokenizer class so much. The Most advantage of Loaden's patch is that he implement a true "expression solver". For example:

Code
//#define AAA

#ifdef AAA
#include <xxx.h>
#else
#include <yyy.h>
#endif


The current SVN code (also the patch supplied by me) only parse the code like below, all the #else branches were skipped whether AAA is defined or not.
Code
//#define AAA
#include <xxx.h>

But with Loaden's patch, the expression value after #if was dynamically evaluated when parsing, and only a true conditioanl preprocessor branch will be parsed.
So, As a conclusion: Loaden's Patch is much better than mine, you should pick his patch.

If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.