Developer forums (C::B DEVELOPMENT STRICTLY!) > CodeCompletion redesign

New code completion remarks/issues

<< < (38/54) > >>

MortenMacFly:

--- Quote from: ollydbg on October 11, 2009, 03:07:39 pm ---Parsing stage done (1511 total parsed files, 84442 tokens in 0 minute(s), 12.468 seconds).
after patch
Parsing stage done (1510 total parsed files, 84506 tokens in 0 minute(s), 12.578 seconds).

--- End quote ---
...did you notice? That's weired.


--- Quote from: ollydbg on October 19, 2009, 04:09:49 pm ---1. In Tokenizer.cpp. the if condition never be true in the DoGetToken function
[...]
So, these code can be deleted.

--- End quote ---
...which one (which if condition) you mean? For me both still make sense...?!


--- Quote from: ollydbg on October 19, 2009, 04:09:49 pm ---2, I tokenizer.h think in the CurrentChar function, we don't need to check index value, because MoveToNextChar already do this.

--- End quote ---
That's true, but I wanted to avoid any issues with API mis-use (meaning calling CurrentChar() before MoveToNextChar() for any obscure reason). I wonder what this would differ in terms of time. Implementing a temporary time measurement would reveal the difference.

ollydbg:

--- Quote from: MortenMacFly on October 19, 2009, 08:57:51 pm ---
--- Quote from: ollydbg on October 11, 2009, 03:07:39 pm ---Parsing stage done (1511 total parsed files, 84442 tokens in 0 minute(s), 12.468 seconds).
after patch
Parsing stage done (1510 total parsed files, 84506 tokens in 0 minute(s), 12.578 seconds).

--- End quote ---
...did you notice? That's weird.

--- End quote ---
Yes, it is weird. I don't know why...


--- Quote from: MortenMacFly on October 19, 2009, 08:57:51 pm ---


--- Quote from: ollydbg on October 19, 2009, 04:09:49 pm ---1. In Tokenizer.cpp. the if condition never be true in the DoGetToken function
[...]
So, these code can be deleted.

--- End quote ---
...which one (which if condition) you mean? For me both still make sense...?!

--- End quote ---

Both the if condition were not be hit if I set breakpoint there.
See my screen shot

I has discussed this with blueshake, he can confirm this. That's because in the logic bug in SkipUnwanted.
see the code:

--- Code: ---bool Tokenizer::SkipUnwanted()
{
    while (CurrentChar() == '#' ||
            (!m_IsOperator && CurrentChar() == '=') ||
            (!m_IsOperator && CurrentChar() == '[') ||
            CurrentChar() == '?' ||
            (CurrentChar() == '/' && (NextChar() == '/' || NextChar() == '*') ))
    {
        bool skipPreprocessor = false; // used for #include
        while (m_Buffer.Mid(m_TokenIndex, 2) == _T("//") ||
                m_Buffer.Mid(m_TokenIndex, 2) == _T("/*"))
        {
            // C/C++ style comments
            SkipComment();
            if (IsEOF())
                return false;
            if (!SkipWhiteSpace())
                return false;
        }

        while (CurrentChar() == '#')
        {
            // preprocessor directives
            // we only care for #include and #define, for now
            unsigned int backupIdx = m_TokenIndex;
            MoveToNextChar();
            SkipWhiteSpace();
            if ((CurrentChar() == 'i' && NextChar() == 'n') || // in(clude)
                (CurrentChar() == 'i' && NextChar() == 'f') || // if(|def|ndef)
                (CurrentChar() == 'e' && NextChar() == 'l') || // el(se|if)
                (CurrentChar() == 'e' && NextChar() == 'n') || // en(dif)
                (m_TokenizerOptions.wantPreprocessor && CurrentChar() == 'd' && NextChar() == 'e')) // de(fine)
            {
                // ok, we have something like #in(clude)
                m_LastWasPreprocessor = true;
                m_LastPreprocessor.Clear();
                m_TokenIndex = backupIdx; // keep #
                skipPreprocessor = true;
                break;
            }

--- End code ---

MortenMacFly:

--- Quote from: ollydbg on October 20, 2009, 03:29:50 am ---I has discussed this with blueshake, he can confirm this. That's because in the logic bug in SkipUnwanted.
see the code:

--- End quote ---
So... now you got me a little confused (or probably it's too early for me). So you propose to remove the "if" conditions because of a bug in SkipUnwanted? Shouldn't we better fix the bug? And then: Where exactly is the bug in the code snippet?
Could you do me a favour and comment the lines / snippets in question in the code you've posted? That'll help me a lot. Please explain a little more... :(

ollydbg:

--- Quote from: MortenMacFly on October 20, 2009, 07:52:43 am ---
--- Quote from: ollydbg on October 20, 2009, 03:29:50 am ---I has discussed this with blueshake, he can confirm this. That's because in the logic bug in SkipUnwanted.
see the code:

--- End quote ---
So... now you got me a little confused (or probably it's too early for me). So you propose to remove the "if" conditions because of a bug in SkipUnwanted? Shouldn't we better fix the bug? And then: Where exactly is the bug in the code snippet?
Could you do me a favour and comment the lines / snippets in question in the code you've posted? That'll help me a lot. Please explain a little more... :(

--- End quote ---

Ok, let me explain:

There are two member variables:

m_LastWasPreprocessor is a bool
m_LastPreprocessor is a wxString

But the DoGetToken() only return the variable "str"


--- Code: ---    if (    m_LastWasPreprocessor
        && !str.IsSameAs(_T("#"))
        && !m_LastPreprocessor.IsSameAs(_T("#")) )
    {
        if (!m_LastPreprocessor.IsSameAs(TokenizerConsts::include_str))
        {
            // except for #include and #if[[n]def], all other preprocessor directives need only
            // one word exactly after the directive, e.g. #define THIS_WORD
            SkipToEOL();
        }
        m_LastPreprocessor.Clear();
    }

    if (m_LastWasPreprocessor)
        m_LastPreprocessor << str;

    m_LastWasPreprocessor = false;

    return str;

--- End code ---

So, str will never be affected by m_LastWasPreprocessor  or m_LastPreprocessor.

So, these two member variables are useless.


blueshake:
what more I want to say is the logical error:
The only chance the variable m_LastWasPreprocessor  becomes true is in these codes:

--- Code: ---            if ((CurrentChar() == 'i' && NextChar() == 'n') || // in(clude)
                (CurrentChar() == 'i' && NextChar() == 'f') || // if(|def|ndef)
                (CurrentChar() == 'e' && NextChar() == 'l') || // el(se|if)
                (CurrentChar() == 'e' && NextChar() == 'n') || // en(dif)
                (m_TokenizerOptions.wantPreprocessor && CurrentChar() == 'd' && NextChar() == 'e')) // de(fine)
            {
                // ok, we have something like #in(clude)
                m_LastWasPreprocessor = true;
                m_LastPreprocessor.Clear();
                m_TokenIndex = backupIdx; // keep #
                skipPreprocessor = true;
                break;
            }
            else
--- End code ---

at this time,str contain the string "#",and m_LastPreprocessor is empty.(see codes above)
so the three conditions in if:

m_LastWasPreprocessor------------------------true
!str.IsSameAs(_T("#"))-------------------------false
!m_LastPreprocessor.IsSameAs(_T("#"))-------true

see ,the three conditions can not all be true at the same time.It means the "if " condition can never be true.

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version