Check macro usage on every identifier like token: issues and discussions

[ollydbg]

Code

void Function1(void)
{
    int some_int;
    for (int for_int = 0; for_int < 5; ++for_int)
    {
        //  //some_int
        //for   //for_int
    }
}

void Function2(void)
{
    int value1 = 0;
    int value2 = 0;

    // hover on "value1"
    if (!value1) {}
    // hover on "value1" and "value2"
    if (value1 && value2) {}

    // Above tooltips will be shown correctly because the statements
    // within if(...), do(...), etc are NOT parsed entirely.
    // Revision 10230 "fixes" this problem, but now, parsing these statements
    // introduces the following regression.
    //if (!value1) {}   // adds a new token "! value1"
    //if (value1 && value2){}   // adds a new token "value1&& value2"
}

It looks like if (xxx), where xxx is an expression.
and the Code::Blocks / Tickets / #145 Code completion ignores parameters of catch-clauses, the test code I use:

Code

class E
{
    int a;
};

int main()
{
    try
    {
    }
    catch(E exp123)
    {
        exp|
    }
    return 0;
}

Where catch(yyy), here yyy is much like function arguments, which is not an expression.

And the void ParserThread::HandleConditionalArguments() is mainly handling the "xxx", not "yyy". 

[ollydbg]

* CC: [huki] parser - fix for function pointer parsing with assignment.

I think remove the ';' check is not quite good, you just want to support some patters like:

Code

m_Str AAA (*BBB) (...);
m_Str AAA (*BBB) (...) = some_function;

Why not just check if the peek is either ';' or '='?
I think there are other cases which could not lead to a function pointer, such as C's Type Casting.

Code

m_Str AAA (*typeA) (*typeB)(*typeC)....

Or maybe, there are other cases.

[Huki]

* CC: [huki] parser - fix for function pointer parsing with assignment.

I think remove the ';' check is not quite good, you just want to support some patters like:

Code

m_Str AAA (*BBB) (...);
m_Str AAA (*BBB) (...) = some_function;

Why not just check if the peek is either ';' or '='?
I think there are other cases which could not lead to a function pointer, such as C's Type Casting.

Code

m_Str AAA (*typeA) (*typeB)(*typeC)....

Or maybe, there are other cases.

I agree, checking for both ';' and '=' should be ok. Modified patch:

Code

From f01ada3e356006475787c481316f405ab47359f9 Mon Sep 17 00:00:00 2001
From: huki <gk7huki@gmail.com>
Date: Wed, 8 Apr 2015 08:12:47 +0530
Subject: [PATCH 15/17] CC: [huki] parser - fix for function pointer parsing
 with assignment.

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

diff --git a/src/plugins/codecompletion/parser/parserthread.cpp b/src/plugins/codecompletion/parser/parserthread.cpp
index 813bdd5..d3e6cdd 100644
--- a/src/plugins/codecompletion/parser/parserthread.cpp
+++ b/src/plugins/codecompletion/parser/parserthread.cpp
@@ -2227,7 +2227,7 @@ void ParserThread::HandleFunction(wxString& name, bool isOperator, bool isPointe
         int pos = name.find(ParserConsts::ptr);
 
         // pattern: m_Str AAA (*BBB) (...);
-        if (peek == ParserConsts::semicolon && pos != wxNOT_FOUND)
+        if (pos != wxNOT_FOUND && (peek == ParserConsts::semicolon || peek == ParserConsts::equals))
         {
             name.RemoveLast();  // remove ")"
             name.Remove(0, pos+1).Trim(false); // remove "(* "
@@ -2235,7 +2235,7 @@ void ParserThread::HandleFunction(wxString& name, bool isOperator, bool isPointe
             // pattern: m_Str AAA (*BBB[X][Y]) (...);
             pos = name.find(ParserConsts::oparray_chr);
             if (pos != wxNOT_FOUND)
-                name.Remove(pos);
+                name.Remove(pos).Trim(true);
 
             TRACE(_T("HandleFunction() : Add token name='")+name+_T("', args='")+args+_T("', return type='") + m_Str+ _T("'"));
             Token* newToken =  DoAddToken(tkFunction, name, lineNr, 0, 0, args);
-- 
2.1.4

I added the Trim(true) for safety, in case the name contains a trailing space. But I see you already handled that case in the tokenizer's ReadParentheses (see your commit). So feel free to ignore this part.
Then, the only change is to test for both ";" and "=".

[Huki]

Code

void Function2(void)
{
    int value1 = 0;
    int value2 = 0;

    // hover on "value1"
    if (!value1) {}
    // hover on "value1" and "value2"
    if (value1 && value2) {}

    // Above tooltips will be shown correctly because the statements
    // within if(...), do(...), etc are NOT parsed entirely.
    // Revision 10230 "fixes" this problem, but now, parsing these statements
    // introduces the following regression.
    //if (!value1) {}   // adds a new token "! value1"
    //if (value1 && value2){}   // adds a new token "value1&& value2"
}

It looks like if (xxx), where xxx is an expression.
and the Code::Blocks / Tickets / #145 Code completion ignores parameters of catch-clauses, the test code I use:

Code

class E
{
    int a;
};

int main()
{
    try
    {
    }
    catch(E exp123)
    {
        exp|
    }
    return 0;
}

Where catch(yyy), here yyy is much like function arguments, which is not an expression.

And the void ParserThread::HandleConditionalArguments() is mainly handling the "xxx", not "yyy". 

Then we can add a parameter "bool parseArguments" to HandleConditionalArguments(), which will be false by default for 'if', 'do', 'while' and can be set to true to handle catch expressions. Is that ok with you?

[ollydbg]

...
Then we can add a parameter "bool parseArguments" to HandleConditionalArguments(), which will be false by default for 'if', 'do', 'while' and can be set to true to handle catch expressions. Is that ok with you?

I totally agree on this idea. Thanks.

...
I agree, checking for both ';' and '=' should be ok. Modified patch:
...
I added the Trim(true) for safety, in case the name contains a trailing space. But I see you already handled that case in the tokenizer's ReadParentheses (see your commit). So feel free to ignore this part.
Then, the only change is to test for both ";" and "=".

Thanks for the modified patch, I'm OK with this change. This is what I'm going to commit:

Code

From 23346fed9a26273fbde0f9f859cac9da0e31ec2d Mon Sep 17 00:00:00 2001
From: huki <gk7huki@gmail.com>
Date: Wed, 8 Apr 2015 08:12:47 +0530
Subject: * CC: parser - fix for function pointer parsing with assignment.
 (Thanks Huki)

Parsing code pattern:
m_Str AAA (*BBB) (...) = some_function;
See discussion here:
http://forums.codeblocks.org/index.php/topic,19769.msg140852.html#msg140852

diff --git a/src/plugins/codecompletion/parser/parserthread.cpp b/src/plugins/codecompletion/parser/parserthread.cpp
index 813bdd5..d86cfee 100644
--- a/src/plugins/codecompletion/parser/parserthread.cpp
+++ b/src/plugins/codecompletion/parser/parserthread.cpp
@@ -2227,15 +2227,17 @@ void ParserThread::HandleFunction(wxString& name, bool isOperator, bool isPointe
         int pos = name.find(ParserConsts::ptr);
 
         // pattern: m_Str AAA (*BBB) (...);
-        if (peek == ParserConsts::semicolon && pos != wxNOT_FOUND)
+        // pattern: m_Str AAA (*BBB) (...) = some_function;
+        if (pos != wxNOT_FOUND && (peek == ParserConsts::semicolon || peek == ParserConsts::equals))
         {
             name.RemoveLast();  // remove ")"
             name.Remove(0, pos+1).Trim(false); // remove "(* "
 
             // pattern: m_Str AAA (*BBB[X][Y]) (...);
+            // Trim(true) for safety, in case the name contains a trailing space
             pos = name.find(ParserConsts::oparray_chr);
             if (pos != wxNOT_FOUND)
-                name.Remove(pos);
+                name.Remove(pos).Trim(true);
 
             TRACE(_T("HandleFunction() : Add token name='")+name+_T("', args='")+args+_T("', return type='") + m_Str+ _T("'"));
             Token* newToken =  DoAddToken(tkFunction, name, lineNr, 0, 0, args);

[ollydbg]

CC: [huki] parser - fixed handling of assignment within for loop.

I'm OK with this commit.
With this patch, we can handle the pattern:

Code

for(int i = 0; ...; ...)...

And correctly recognize "i" as a variable.
I will commit soon. This is an enhancement on the original patch, see revision 8700, and the related patch page: Patch 3345 - Code::Blocks History

But I see in the function void ParserThread::HandleForLoopArguments()'s body:

Code

...
        if (peek == ParserConsts::comma)
        {
            smallTokenizer.GetToken(); // eat comma
            createNewToken = true;
        }
        else if (peek == ParserConsts::colon
                 || peek == ParserConsts::semicolon
                 || peek.empty())
        {
            createNewToken = true;
            finished = true; // after this point there will be no further declarations
        }
...

It looks like handling colon is not quite correct. For example:

Code

for ( SomeNamespace::SomeClass i = 0; ...)

In this case, it should add a token "i", which the type "SomeNamespace::SomeClass".
The current method is try to construct a small buffer(smallTokenizer), which contains:

Code

( SomeNamespace::SomeClass i = 0; ...)

and run some parsing method on it.

My idea could be:
1, Make the buffer "( SomeNamespace::SomeClass i = 0; ...)", and construct a new Parserthread object
2, call DoParse() function recursively on the buffer, which is quite similar with how we parse the class definitions

Code

class AAA
{
     // body of the class definition, we recursively to call the DoParse() function.
     // So that all tokens are marked as the child token of the Token "AAA".
     // once the "}" is met, the Doparse() will returned to its high level DoParse().
}

3, if possible, we can let the Token "i" be a child Token of the current Token(ideally the Token "for"), although all the tokens are temporary tokens.

In the main loop of the DoParse(), the "SomeNamespace::SomeClass i = 0;" will be correctly recognized as a Token "i".
This is the way we can handle function parameters, or the catch(xxx) clause, as we have already discussed.

[ollydbg]

@Huki, your patches are in trunk now, thanks.