Is it possible for the parser to support newlib prototypes?

[ollydbg]

I'm testing your patch now. It looks like macro expansion is enabled on every identifier like tokens.

Yes, when the (!switchHandled) case is reached in DoParse(), it now expands all macros, including variable-like ones.

When I first look at this function, I see that it only expand function-like macro usage.

Code

void ParserThread::HandleMacroExpansion(int id, const wxString &peek)
{
    Token* tk = m_TokenTree->at(id);
    if (tk)
    {
        TRACE(_T("HandleMacroExpansion() : Adding token '%s' (peek='%s')"), tk->m_Name.wx_str(), peek.wx_str());
        DoAddToken(tkMacroUse, tk->m_Name, m_Tokenizer.GetLineNumber(), 0, 0, peek);

        if (m_Options.parseComplexMacros)
            m_Tokenizer.ReplaceFunctionLikeMacro(tk);
    }
}

But This is not true.

Code

bool Tokenizer::ReplaceFunctionLikeMacro(const Token* tk)
{
    wxString macroExpandedText;
    if ( GetMacroExpandedText(tk, macroExpandedText) )
        return ReplaceBufferText(macroExpandedText);
    return false;
}

Our recent changes in GetMacroExpandedText() function covers the handling of variable-like macro usage expansion.

So, the function name "ReplaceFunctionLikeMacro" cause confusing and need to be renamed. 

[ollydbg]

...
On second look I noticed we can do a little optimization. This code:

Code

wxString arg = peek;
arg.Remove(0,1); // remove '('
if (arg.GetChar(0) == ParserConsts::ptr)
{
    arg.Remove(0,1).Trim(false); // remove '*'
    [...]
}

can be changed to:

Code

if (peek.GetChar(1) == ParserConsts::ptr)
{
    wxString arg = peek;
    arg.Remove(0,2).Trim(false); // remove "(*"
    [...]
}

So the entire code for this case:

Code

// see what is inside the (...)
// try to see whether the peek pattern is (* BBB)
if (peek.GetChar(1) == ParserConsts::ptr)
{
    wxString arg = peek;
    arg.RemoveLast(); // remove ")"
    arg.Remove(0,2).Trim(false); // remove "(* "
    m_Str << token;
    token = m_Tokenizer.GetToken(); //consume the peek
    // BBB is now the function ptr's name
    HandleFunction(/*function name*/ arg,
                   /*isOperator*/    false,
                   /*isPointer*/     true);
}
else if [...]

That way we don't have to create a temporary variable 'arg' every time when we see a function declaration-like pattern, but only when we know for sure it can be a function pointer.

I'm testing your patch now. It looks like macro expansion is enabled on every identifier like tokens.

Yes, when the (!switchHandled) case is reached in DoParse(), it now expands all macros, including variable-like ones.

Hi, Huki' patch is in trunk now, thanks for your contribution!

[ollydbg]

...
So, the function name "ReplaceFunctionLikeMacro" cause confusing and need to be renamed. 

Done in the trunk r9934.

[WinterMute]

Wow! Amazing job. Thank you so much for doing this - I really wasn't expecting it 

[Huki]

Hi,
I reviewed some remaining patches I have and noticed I have not yet submitted this one:

Oops yeah, I confirm, it works like in your screenshot, good job !

(Minor UI detail, it would be nice if the function calltip was showing the true name of the function and not PFNGLDRAWARRAYSINSTANCEDPROC)

Yes, I think it would be better to show the variable name rather than the typedef name. See here:


I'll submit the patch soon...

When we display the calltip for a typedef'd function pointer, we can show the function pointer's name rather than the typedef's name.
Here is the patch:

Code

From 65004d39e614519c66475a5295a6f6d44e4e3341 Mon Sep 17 00:00:00 2001
From: huki <gk7huki@gmail.com>
Date: Thu, 18 Sep 2014 18:25:20 +0530
Subject: CC: update typedef'd func pointer calltip

---
 src/plugins/codecompletion/nativeparser_base.cpp | 47 +++++++++++++-----------
 src/plugins/codecompletion/nativeparser_base.h   |  8 ++--
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/src/plugins/codecompletion/nativeparser_base.cpp b/src/plugins/codecompletion/nativeparser_base.cpp
index 778d55a..5abfadb 100644
--- a/src/plugins/codecompletion/nativeparser_base.cpp
+++ b/src/plugins/codecompletion/nativeparser_base.cpp
@@ -1685,7 +1685,7 @@ void NativeParserBase::ComputeCallTip(TokenTree*         tree,
 
             // either a function or a variable, but it is OK if a macro with not empty m_Args.
             if (tk && ((tk->m_TokenKind ^ tkMacroDef) || !tk->m_Args.empty()))
-                token = tk; // tkVariable could be a typedef'd function ptr (checked later down below)
+                token = tk; // tkVariable could be a typedef'd function ptr (checked in PrettyPrintToken())
             else
             {
                 // a variable like macro, this token don't have m_Args(call tip information), but
@@ -1711,33 +1711,36 @@ void NativeParserBase::ComputeCallTip(TokenTree*         tree,
             }
         }
 
-        // a variable basically don't have call tips, but if it's type is a typedef'd function
-        // pointer, we can still have call tips (which is the typedef function's arguments)
-        if (token->m_TokenKind == tkVariable)
-        {
-            const Token* tk = tree->at(tree->TokenExists(token->m_BaseType, token->m_ParentIndex, tkTypedef));
-            if (!tk && token->m_ParentIndex != -1)
-                tk = tree->at(tree->TokenExists(token->m_BaseType, -1, tkTypedef));
-            if (tk && !tk->m_Args.empty())
-                token = tk; // typedef'd function pointer
-        }
+        wxString tkTip;
+        if ( !PrettyPrintToken(tree, token, tkTip) )
+            tkTip = wxT("Error while pretty printing token!");
+        items.Add(tkTip);
 
-        {
-            wxString tkTip;
-            if ( !PrettyPrintToken(tree, token, tkTip) )
-                tkTip = wxT("Error while pretty printing token!");
-            items.Add(tkTip);
-        }
     }// for
 
     CC_LOCKER_TRACK_TT_MTX_UNLOCK(s_TokenTreeMutex)
 }
 
-bool NativeParserBase::PrettyPrintToken(const TokenTree*  tree,
-                                        const Token*      token,
-                                        wxString&         result,
-                                        bool              isRoot)
+bool NativeParserBase::PrettyPrintToken(TokenTree*      tree,
+                                        const Token*    token,
+                                        wxString&       result,
+                                        bool            isRoot)
 {
+    wxString name = token->m_Name;
+    // a variable basically don't have call tips, but if it's type is a typedef'd function
+    // pointer, we can still have call tips (which is the typedef function's arguments)
+    if (token->m_TokenKind == tkVariable)
+    {
+        const Token* tk = tree->at(tree->TokenExists(token->m_BaseType, token->m_ParentIndex, tkTypedef));
+        if (!tk && token->m_ParentIndex != -1)
+            tk = tree->at(tree->TokenExists(token->m_BaseType, -1, tkTypedef));
+        if (tk && !tk->m_Args.empty()) // typedef'd function pointer
+        {
+            name = token->m_Name;
+            token = tk;
+        }
+    }
+
     // if the token has parents and the token is a container or a function,
     // then pretty print the parent of the token->
     if (   (token->m_ParentIndex != -1)
@@ -1776,7 +1779,7 @@ bool NativeParserBase::PrettyPrintToken(const TokenTree*  tree,
             return true;
 
         case tkTypedef:
-            result = token->m_BaseType + wxT(" ") + result + token->m_Name + token->GetFormattedArgs();
+            result = token->m_BaseType + wxT(" ") + result + name + token->GetFormattedArgs();
             return true;
 
         case tkEnum:
diff --git a/src/plugins/codecompletion/nativeparser_base.h b/src/plugins/codecompletion/nativeparser_base.h
index 586c5ae..ab90894 100644
--- a/src/plugins/codecompletion/nativeparser_base.h
+++ b/src/plugins/codecompletion/nativeparser_base.h
@@ -321,10 +321,10 @@ protected:
     /** For ComputeCallTip()
      * No critical section needed in this recursive function!
      * All functions that call this recursive function, should already entered a critical section. */
-    bool PrettyPrintToken(const TokenTree*   tree,
-                          const Token*       token,
-                          wxString&          result,
-                          bool               isRoot = true);
+    bool PrettyPrintToken(TokenTree*    tree,
+                          const Token*  token,
+                          wxString&     result,
+                          bool          isRoot = true);
 
     // convenient static funcs for fast access and improved readability
 
-- 
1.9.4.msysgit.0

PS: I got your notification about checking macro usage for every token, but I was too busy to check it earlier. Anyway, it's too complex so I think it's better to keep it aside for now...

[ollydbg]

@Huki, sorry for the late reply, we are in spring festival now. 
Happy Chinese new year to all guys!!!
You patch is in trunk now (rev 10124), thanks. I see that keeping the const qualifier in the function prototype needs a lot of work, so simply follow your way to remove the const.

About the macro replacement improvement, we have new serial patches, see: Macro expansion infinite loop.. I tried to apply your patch on top of my macro replacement improvement serials, but the result is not expected as on trunk. Not sure why...

[Huki]

Here is a patch attached, to support array of function pointer such as:

Code

int (*FuncArray[X][Y]) (...);

In the case of function pointers, we now call HandleFunction() with "name" argument in the form (*BBB) or (*BBB[X][Y]...). Then we strip the () * [] inside HandleFunction() to extract BBB.

[Huki]

Some fixes in HandleTypedef() for typedef pointers:
When viewing tooltips for struct pointer typedefs (support added in r10155), we see a missing space and stray ")". From src/plugins/codecompletion/testing/cc_typedef_pointer.cpp:

Code

typedef struct foo1 * foo1Ptr;

hover on "foo1Ptr" and we see:

Code

typedef foo1foo1Ptr)

Fixed with this patch:

Code

@@ -3103,6 +3110,8 @@ bool ParserThread::ReadClsNames(wxString& ancestor)
             m_Str.clear();
             m_Str = ancestor;
 
+            m_PointerOrRef.Clear(); // FIX: for typedef pointers
+
             // Detects anonymous ancestor and gives him a name based on the first found alias.
             if (m_Str.StartsWith(g_UnnamedSymbol))
             {

The entire patch is attached below.

[Huki]

About the macro replacement improvement, we have new serial patches, see: Macro expansion infinite loop.. I tried to apply your patch on top of my macro replacement improvement serials, but the result is not expected as on trunk. Not sure why...

I'm currently testing the cc_macro serial patches and it's very stable so far. I'm fixing several bugs, the one mentioned in the above quote is fixed too.

[ollydbg]

About the macro replacement improvement, we have new serial patches, see: Macro expansion infinite loop.. I tried to apply your patch on top of my macro replacement improvement serials, but the result is not expected as on trunk. Not sure why...

I'm currently testing the cc_macro serial patches and it's very stable so far. I'm fixing several bugs, the one mentioned in the above quote is fixed too.

Hi, thanks for the testing!
I just tested your two patches.
1, patch handling array of function pointer, this works fine! I also add the test case to our cctest.
2, patch handling typedef pointers. I have some questions:

The token is like below before and after your patch:

Code

before

$3 = {m_FullType = L"foo1*", m_BaseType = L"foo1", m_Name = L"foo1Ptr", m_Args = <g_strEmpty+12> L"", m_BaseArgs = <g_strEmpty+12> L"", m_AncestorsString = <g_strEmpty+12> L"", m_FileIdx = 1, m_Line = 9, m_ImplFileIdx = 0, m_ImplLine = 0, m_ImplLineStart = 0, m_ImplLineEnd = 0, m_Scope = tsUndefined, m_TokenKind = tkTypedef, m_IsOperator = false, m_IsLocal = true, m_IsTemp = false, m_IsConst = false, m_IsNoExcept = false, m_IsAnonymous = false, m_Index = 248, m_ParentIndex = -1
, m_Children = std::set with 0 elements, m_Ancestors = std::set with 2 elements = {[0] = 245, [1] = 247}, m_DirectAncestors = std::set with 2 elements = {[0] = 245, [1] = 247}, m_Descendants = std::set with 0 elements, m_Aliases = wxArray<T>, m_TemplateArgument = <g_strEmpty+12> L"", m_TemplateType = wxArray<T>, m_TemplateMap = std::map with 0 elements, m_TemplateAlias = <g_strEmpty+12> L"", m_UserData = 0x0, m_TokenTree = 0x9a7d020, m_Ticket = 504}


After

$1 = {m_FullType = L"foo1"
, m_BaseType = L"foo1", m_Name = L"foo1Ptr", m_Args = <g_strEmpty+12> L"", m_BaseArgs = <g_strEmpty+12> L"", m_AncestorsString = <g_strEmpty+12> L"", m_FileIdx = 1, m_Line = 9, m_ImplFileIdx = 0, m_ImplLine = 0, m_ImplLineStart = 0, m_ImplLineEnd = 0, m_Scope = tsUndefined, m_TokenKind = tkTypedef, m_IsOperator = false, m_IsLocal = true, m_IsTemp = false, m_IsConst = false, m_IsNoExcept = false, m_IsAnonymous = false, m_Index = 248, m_ParentIndex = -1, m_Children = std::set with 0 elements, m_Ancestors = std::set with 2 elements = {[0] = 245, [1] = 247}, m_DirectAncestors = std::set with 2 elements = {[0] = 245, [1] = 247}, m_Descendants = std::set with 0 elements, m_Aliases = wxArray<T>, m_TemplateArgument = <g_strEmpty+12> L""
, m_TemplateType = wxArray<T>, m_TemplateMap = std::map with 0 elements, m_TemplateAlias = <g_strEmpty+12> L""
, m_UserData = 0x0, m_TokenTree = 0x9a6aa20, m_Ticket = 504}

Do you think we need to remove the "*" from the "m_FullType"?

I just debugged a little, and I found this code snippet:

Code

wxString Token::DisplayName() const
{
    wxString result;
    if      (m_TokenKind == tkClass)
        return result << _T("class ")     << m_Name << m_BaseArgs << _T(" {...}");
    else if (m_TokenKind == tkNamespace)
        return result << _T("namespace ") << m_Name << _T(" {...}");
    else if (m_TokenKind == tkEnum)
        return result << _T("enum ")      << m_Name << _T(" {...}");
    else if (m_TokenKind == tkTypedef)
    {
        result << _T("typedef");

        if (!m_FullType.IsEmpty())
            result << _T(" ") << m_FullType;

        if (result.Find('*', true) != wxNOT_FOUND)
        {
            result.RemoveLast();
            return result << m_Name << _T(")") <<  GetFormattedArgs();
        }

        if (!m_TemplateArgument.IsEmpty())
            result << m_TemplateArgument;

        return result << _T(" ") << m_Name;
    }

Question is:
What does the below code's purpose?

Code

        if (result.Find('*', true) != wxNOT_FOUND)
        {
            result.RemoveLast();
            return result << m_Name << _T(")") <<  GetFormattedArgs();
        }

Thanks.

[Huki]

Do you think we need to remove the "*" from the "m_FullType"?

It was the only way I could fix the "typedef foo1foo1Ptr)" tooltip bug, but now you have found the real source of the bug in Token::DisplayName(), so we don't have to  remove the "*" anymore (see below).

Question is:
What does the below code's purpose?

Code

        if (result.Find('*', true) != wxNOT_FOUND)
        {
            result.RemoveLast();
            return result << m_Name << _T(")") <<  GetFormattedArgs();
        }

In HandleTypedef, we support 2 cases of typedef'd function pointers, and in each case the type is stored as below:

Code

typedef void (*dMessageFunction)(int errnum, const char *msg, va_list ap);
// --> type is stored as: (*)

typedef void (MyClass::*Function)(int);
// --> type is stored as: (MyClass::*)

Then in Token::DisplayName() this type is taken as "result". So the code below removes the last ')' and adds the m_Name:

Code

        // result is "typedef void (*)", m_Name is "Function"
        if (result.Find('*', true) != wxNOT_FOUND)
        {
            result.RemoveLast(); // result is "typedef void (*"
            return result << m_Name << _T(")") <<  GetFormattedArgs(); // we return "typedef void (*Function) (args)"
        }

So the bug occurs when we test for "if (result.Find('*', true) != wxNOT_FOUND)", but the typedef is not a function pointer (it could be a struct pointer).
A way to fix it is to ensure we really have ')' as the last char. I suggest changing the test like this:

Code

if (result.Find('*', true) != wxNOT_FOUND && result.Last() == ')')

Then my previous hack to clear m_PointerOrRef is not required.. I quickly tested it and it's working: I get "typedef foo1* foo1Ptr" as expected.

[ollydbg]

Hi, Huki, thanks for the detailed explanation, the modified patches is in trunk now. (I also added two test cases for our CCTest project)