Author Topic: Find Declaration of constructor doesn't work  (Read 36555 times)

Offline GeO

  • Multiple posting newcomer
  • *
  • Posts: 51
Find Declaration of constructor doesn't work
« on: November 25, 2010, 03:15:29 pm »
As the title say, the declaration of wxWidgets constructors (maybe other external constructors too) cannot be found.

For example: if you have this single line and you click "find declaration" on the first wxStaticText* the class wxStaticText will correctly showed,
however if you click on the constructor wxStaticText, the message "Declaration not found" will popup.
Code
wxStaticText* static = new wxStaticText(this, wxID_ANY, label, pos, size, style, name);
   
I'm using the latest package from jens (rev 6853).

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Find Declaration of constructor doesn't work
« Reply #1 on: November 29, 2010, 01:57:40 pm »
I have just test a simple code:
Code
#include <iostream>

using namespace std;

class AAA
{
public:
    AAA(){};
    int a;
};

int main()
{
    AAA * p = new AAA();
    cout << "Hello world!" << endl;
    return 0;
}

I can reproduce this bug.
I will check it.
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: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Find Declaration of constructor doesn't work
« Reply #2 on: November 29, 2010, 02:44:33 pm »
I think the bug was introduced in the rev
Quote
loaden  2010-11-2 22:30:23            
* special handle constructor function for 'Goto declaration' and 'Goto implementation'

I just debug the code to line 2672 of codecompletion.cpp, code snippet below:
Code
        if (isClassOrConstructor)
        {
            const bool isConstructor = (GetNextNonWhitespaceChar(editor->GetControl(), end) == _T('('));
            for (TokenIdxSet::iterator it = result.begin(); it != result.end();)
            {
                Token* tk = tokens->at(*it);
                if (isConstructor && tk && tk->m_TokenKind == tkClass)
                    result.erase(it++);     //line 2672 ---------------------------------------------------------------
                else if (!isConstructor && tk && tk->m_TokenKind == tkConstructor)
                    result.erase(it++);
                else
                    ++it;
            }
        }
    }

the result contains only one token. but the line 2672, it seems the correct token was deleted. Any ideas??? Loaden or others.
« Last Edit: November 29, 2010, 02:59:08 pm by ollydbg »
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: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Find Declaration of constructor doesn't work
« Reply #3 on: November 29, 2010, 02:58:22 pm »
the result token set should have two tokens, they are:
1 class AAA
2 constructor AAA::AAA

but I get only one token, which is "class AAA". so, it seems there is a bug in other place. (not the line 2672 in my previous post).

Edit
it seems the constructor AAA::AAA is the child of class AAA, so it is correct that only one token(class AAA) as the result. So we should do special handling of this kind of "find declaration". :D
« Last Edit: November 29, 2010, 03:35:03 pm by ollydbg »
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 proxym

  • Single posting newcomer
  • *
  • Posts: 3
Re: Find Declaration of constructor doesn't work
« Reply #4 on: January 20, 2014, 04:29:41 pm »
Find Declaration of constructor still doesn't work. 2014 year now.
Instead of showing declaration of constructor it shows declaration of class.
Code::blocks svn 9455.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Find Declaration of constructor doesn't work
« Reply #5 on: January 26, 2014, 04:08:10 am »
Find Declaration of constructor still doesn't work. 2014 year now.
Instead of showing declaration of constructor it shows declaration of class.
Code::blocks svn 9455.
Indeed, it is not fixed. Now, I need some suggestions.

Let's still look at the sample code in this Re: Find Declaration of constructor doesn't work

What is the expect behavior?

Code
#include <iostream>

using namespace std;

class AAA           //-----------------------(a)
{
public:
    AAA(){};        //-----------------------(b)
    int a;
};

int main()
{
    AAA * p = new AAA();
    ^^^----------------------------------------------(1)
                  ^^^-------------------------------(2)

    cout << "Hello world!" << endl;
    return 0;
}

So, here is my rules:
case 1: find declaration in (1) should go to (a), not (b)
case 2: find declaration in (2) should go to (b), not (a)

But
case 3: do you think that find declaration in (1) should go to show a list box showing both (a) and (b), and let the user select where to go?
case 4: do you think that find declaration in (1) should go to show a list box showing both (a) and (b), and let the user select where to go?

I think Alpha has did some work which is quite similar to the solution of this issue, see the SVN commit WebSVN - codeblocks - Rev 9560 - /, but that's only for call-tip.
Question again here, does call-tip need to show a class definition Token? I basically think that a call-tip should show only a function prototype tip, unless this class does not have a constructor.






« Last Edit: January 26, 2014, 04:17:25 am by ollydbg »
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: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Find Declaration of constructor doesn't work
« Reply #6 on: January 26, 2014, 07:28:03 am »
Code
#include <iostream>

using namespace std;

class AAA
{
public:
    AAA(int b){};
    int a;
};

class BBB
{
public:
    int a;
};

int main()
{
    AAA * p = new AAA(6);
    BBB * p = new BBB(6);
    cout << "Hello world!" << endl;
    return 0;
}


The above code is to demonstrate the fix of this issue, you can use the mouse to hover "AAA" or "BBB", and find declaration goes correctly according to the rules:
Quote
case 1: find declaration in (1) should go to (a), not (b)
case 2: find declaration in (2) should go to (b), not (a)

Note that in "BBB", there is no constructor defined, so it go to the class definition.

Here is the patch:
Code
From a204e285017afe0aa41e4660b0eaa2a59504aaed Mon Sep 17 00:00:00 2001
From: asmwarrior
Date: Sun, 26 Jan 2014 14:19:04 +0800
Subject: [PATCH] * CC: find declaration of a class constructor goes to the
 correct constructor functions, not the class definition. This fix a bug
 reported here:
 http://forums.codeblocks.org/index.php/topic,13753.msg92654.html#msg92654

---
 src/plugins/codecompletion/codecompletion.cpp    | 41 ++++++++++--------------
 src/plugins/codecompletion/nativeparser_base.cpp | 36 ++++++++++++++++++++-
 src/plugins/codecompletion/nativeparser_base.h   |  3 ++
 3 files changed, 55 insertions(+), 25 deletions(-)

diff --git a/src/plugins/codecompletion/codecompletion.cpp b/src/plugins/codecompletion/codecompletion.cpp
index ba48475..1fc3af5 100644
--- a/src/plugins/codecompletion/codecompletion.cpp
+++ b/src/plugins/codecompletion/codecompletion.cpp
@@ -2219,34 +2219,27 @@ void CodeCompletion::OnGotoDeclaration(wxCommandEvent& event)
             }
         }
     }
-    // special handle constructor function
-    else
+    else // special handle constructor and functions
     {
-        bool isClassOrConstructor = false;
-        for (TokenIdxSet::const_iterator it = result.begin(); it != result.end(); ++it)
+        // AAA * p = new AAA();
+        // ^^^---------------------------------------------this should go to class definition
+        //               ^^^-------------------------------this should go to functions such as constructors
+        const bool isFunction = CodeCompletionHelper::GetNextNonWhitespaceChar(editor->GetControl(), endPos)   == _T('(');
+        TokenIdxSet savedResult = result;
+        for (TokenIdxSet::const_iterator it = result.begin(); it != result.end();)
         {
             const Token* token = tree->at(*it);
-            if (token && (token->m_TokenKind == tkClass || token->m_TokenKind == tkConstructor))
-            {
-                isClassOrConstructor = true;
-                break;
-            }
-        }
-        if (isClassOrConstructor)
-        {
-            const bool isConstructor = CodeCompletionHelper::GetNextNonWhitespaceChar(editor->GetControl(), endPos)   == _T('(')
-                                    && CodeCompletionHelper::GetLastNonWhitespaceChar(editor->GetControl(), startPos) == _T(':');
-            for (TokenIdxSet::const_iterator it = result.begin(); it != result.end();)
-            {
-                const Token* token = tree->at(*it);
-                if (isConstructor && token && token->m_TokenKind == tkClass)
-                    result.erase(it++);
-                else if (!isConstructor && token && token->m_TokenKind == tkConstructor)
-                    result.erase(it++);
-                else
-                    ++it;
-            }
+            if (isFunction && token && token->m_TokenKind == tkClass)
+                result.erase(it++); // a class token is removed
+            else if (!isFunction && token && token->m_TokenKind == tkConstructor)
+                result.erase(it++); // a constructor token is removed
+            else
+                ++it;
         }
+        // in a special case that a class definition don't have a constructor defined (implicitly defined by compiler)
+        // add shouldn't remove those class type tokens, so we need to restore the saved result
+        if (!result.size())
+            result = savedResult;
     }
 
     // special handle for function overloading
diff --git a/src/plugins/codecompletion/nativeparser_base.cpp b/src/plugins/codecompletion/nativeparser_base.cpp
index ee06770..3b8a02f 100644
--- a/src/plugins/codecompletion/nativeparser_base.cpp
+++ b/src/plugins/codecompletion/nativeparser_base.cpp
@@ -964,11 +964,45 @@ size_t NativeParserBase::ResolveExpression(TokenTree*                  tree,
     }// while
 
     if (!initialScope.empty())
-        result = initialScope;
+    {
+        // if a Token in initialScope is a class, and we need to add its public constructors, this
+        // is useful when we try to find declaration of a constructor, see
+        // http://forums.codeblocks.org/index.php/topic,13753.msg92654.html#msg92654
+        AddConstructors(tree, initialScope, result);
+    }
 
     return result.size();
 }
 
+void NativeParserBase::AddConstructors(TokenTree *tree, const TokenIdxSet& source, TokenIdxSet& dest)
+{
+    for (TokenIdxSet::iterator It = source.begin(); It != source.end(); ++It)
+    {
+        const Token* token = tree->at(*It);
+        if (!token)
+            continue;
+        dest.insert(*It);
+
+        // add constructors of the class type token
+        if (token->m_TokenKind == tkClass)
+        {
+            // loop on its children, add its public constructors
+            for (TokenIdxSet::iterator chIt = token->m_Children.begin();
+                 chIt != token->m_Children.end();
+                 ++chIt)
+            {
+                const Token* tk = tree->at(*chIt);
+                if (   tk && (   tk->m_TokenKind == tkConstructor
+                              || (tk->m_IsOperator && tk->m_Name.EndsWith(wxT("()"))) )
+                    && (tk->m_Scope == tsPublic || tk->m_Scope == tsUndefined) )
+                {
+                    dest.insert(*chIt);
+                }
+            }
+        }
+    }
+}
+
 void NativeParserBase::ResolveOperator(TokenTree*         tree,
                                        const OperatorType& tokenOperatorType,
                                        const TokenIdxSet&  tokens,
diff --git a/src/plugins/codecompletion/nativeparser_base.h b/src/plugins/codecompletion/nativeparser_base.h
index fc25c04..b2cff33 100644
--- a/src/plugins/codecompletion/nativeparser_base.h
+++ b/src/plugins/codecompletion/nativeparser_base.h
@@ -449,6 +449,9 @@ private:
         return false;
     }
 
+    /**  loop on the source Token set, and add all its public constructors to dest */
+    void AddConstructors(TokenTree *tree, const TokenIdxSet& source, TokenIdxSet& dest);
+
     // for GenerateResultSet()
     bool MatchText(const wxString& text, const wxString& target, bool caseSens, bool isPrefix)
     {
--
1.8.5.2.msysgit.0


I add a function named AddConstructors, which add all the constructor tokens of a class type to the result. This function will be called in the final stage of the NativeParserBase::ResolveExpression().

Comments?  Maybe, Alpha's code in rev9560 is redundant after my 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.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: Find Declaration of constructor doesn't work
« Reply #7 on: January 26, 2014, 07:33:44 pm »
Comments?
You know mine: Cannot try as the patch does not apply. :)
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Find Declaration of constructor doesn't work
« Reply #8 on: January 26, 2014, 07:46:30 pm »
Probably it is time to tryout git-svn:)
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline oBFusCATed

  • Developer
  • Lives here!
  • *****
  • Posts: 13413
    • Travis build status
Re: Find Declaration of constructor doesn't work
« Reply #9 on: January 26, 2014, 07:50:59 pm »
There is a translation that should apply with svn: http://cmpt.benbmp.org/codeblocks/patches/cc.ollydbg.svn.patch

ollydbg: your git patch is missing the email address and fails to apply cleanly with git am, please fix it.
(most of the time I ignore long posts)
[strangers don't send me private messages, I'll ignore them; post a topic in the forum, but first read the rules!]

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Find Declaration of constructor doesn't work
« Reply #10 on: January 27, 2014, 02:31:13 am »
There is a translation that should apply with svn: http://cmpt.benbmp.org/codeblocks/patches/cc.ollydbg.svn.patch
Thanks for the conversion.
Quote
ollydbg: your git patch is missing the email address and fails to apply cleanly with git am, please fix it.
I just manually delete the email address to anti-SPAM, ;). I will use a dummy email address next time.
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 Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Find Declaration of constructor doesn't work
« Reply #11 on: March 03, 2014, 12:04:50 am »
Yes, I have one, I just search our forum for several minutes, and finally found one: Re: Find Declaration of constructor doesn't work.
Yes, thats the one I recall. Well - if Hukis patch provides the same results I would vote for that instead, because it looks a bit simpler.
I see ollydbg's patch is trying to handle both the "go to declaration" and "go to implementation" cases, so it sounds like a more complete solution (my patch only fixes the "go to impl" case).

@ollydbg:
But what does the AddConstructors() do? With my current patch I already get the "Multiple matches" dialog when using "go to impl" on the below code:
Code
cbEditor* cbedpp = new cbEditor(par, file, name, theme);
                       ^^^^^^^^

IMO we just need these 2 changes in CodeCompletion::OnGotoDeclaration:
1) Change:
Code
const bool isConstructor = CodeCompletionHelper::GetNextNonWhitespaceChar(editor->GetControl(), endPos) == _T('(')
     && CodeCompletionHelper::GetLastNonWhitespaceChar(editor->GetControl(), startPos) == _T(':');
to:
Code
const bool isConstructor = CodeCompletionHelper::GetNextNonWhitespaceChar(editor->GetControl(), endPos) == _T('(');
2) Make sure we haven't erased all results:
Code
       // in a special case that a class definition don't have a constructor defined (implicitly defined by compiler)
       // add shouldn't remove those class type tokens, so we need to restore the saved result
       if (!result.size())
           result = savedResult;

Your patch has addressed both these points (and some cleanup for isClassOrConstructor, isConstructor, etc), so it's ok for me. I'm just not sure if the AddConstructors() part is required. Let me know what you think.
« Last Edit: March 03, 2014, 08:36:15 am by Huki »

Offline Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Find Declaration of constructor doesn't work
« Reply #12 on: March 05, 2014, 06:38:59 am »
Let's still look at the sample code in this Re: Find Declaration of constructor doesn't work

What is the expect behavior?

Code
#include <iostream>

using namespace std;

class AAA           //-----------------------(a)
{
public:
    AAA(){};        //-----------------------(b)
    int a;
};

int main()
{
    AAA * p = new AAA();
    ^^^----------------------------------------------(1)
                  ^^^-------------------------------(2)

    cout << "Hello world!" << endl;
    return 0;
}

So, here is my rules:
case 1: find declaration in (1) should go to (a), not (b)
case 2: find declaration in (2) should go to (b), not (a)

But
case 3: do you think that find declaration in (1) should go to show a list box showing both (a) and (b), and let the user select where to go?
Ok, I think your AddConstructors() code was trying to achieve "case 3". My opinion is that only case 1 and 2 are required (i.e., find decl in (1) should only go to (a), the struct name).
So I suggest committing only the changes in CodeCompletion::OnGotoDeclaration of your patch, if you have no objections.


I think Alpha has did some work which is quite similar to the solution of this issue, see the SVN commit WebSVN - codeblocks - Rev 9560 - /, but that's only for call-tip.
Question again here, does call-tip need to show a class definition Token? I basically think that a call-tip should show only a function prototype tip, unless this class does not have a constructor.
In fact I have some improvements for the calltips (typedef and macro functions), I'll test Alpha's patch when I'm going to submit those.

EDIT: BTW, I already get calltips for code like this:
Code
wxStaticText* static = new wxStaticText(|
I think Alpha's patch was trying to get calltips for:
Code
wxStaticText static(|
Though I'm not sure if it can work, because the variable "static" in the above code won't be parsed unless semi-colon is typed, right?
« Last Edit: March 05, 2014, 06:46:33 am by Huki »

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5910
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: Find Declaration of constructor doesn't work
« Reply #13 on: March 06, 2014, 01:52:50 am »
Hi, Huki, sorry for the late reply, I'm busy those days. Let me explain something after I re-read the patch post months ago.

First, the function AddConstructors() is not going to handle the case 3, it is going to handle when user is not explicit define a constructor, such as
Code
class BBB
{
public:
    int a;
};
Now, we have some code like:
Code
BBB * p = new BBB(6);
Now, user hover on the "BBB(6)", then generally, a search TokenTree will happen.
But note that we use the keyword "BBB", also, we search under the scope (global namespace and main() function body), the search result contains only one Token, which is the "class Token BBB". But what a user want is "go to function declaration of BBB()", so there is not such kind of Token in the result. So, as a fallback, we go to the class definition Token (class Token BBB).

Another case is:
Code
class AAA
{
public:
    AAA(int b){};
    int a;
};
int main()
{
AAA * p = new AAA(6);
When user hover on "AAA(6)", we also do a search by keyword "AAA" in our Token (under the global scope and main() function body), then still you get one result "class Token AAA". Note here the constructor AAA is not in the result, because the constructor is a child Token of the class Token AAA. As a general rule, if an identifier is followed by a '(', this gives a hint that user need to go to some function like Tokens. So, I add the function AddConstructors(), which in the last stage of ResolveExpression(). So that both "class Token AAA" and "constructor AAA" is returned.

Later, "class Token AAA" is removed from the result, so we go to the correct declaration of the constructor.

I'm not sure my patch is going to handle "go to implementation xxxxx", I just think it solve some "go to declaration xxxx" issue.
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 Huki

  • Multiple posting newcomer
  • *
  • Posts: 95
Re: Find Declaration of constructor doesn't work
« Reply #14 on: March 07, 2014, 04:08:35 am »
Hi, Huki, sorry for the late reply, I'm busy those days.
No problem.

Code
class AAA
{
public:
    AAA(int b){};
    int a;
};
int main()
{
AAA * p = new AAA(6);
When user hover on "AAA(6)", we also do a search by keyword "AAA" in our Token (under the global scope and main() function body), then still you get one result "class Token AAA". Note here the constructor AAA is not in the result, because the constructor is a child Token of the class Token AAA. As a general rule, if an identifier is followed by a '(', this gives a hint that user need to go to some function like Tokens. So, I add the function AddConstructors(), which in the last stage of ResolveExpression(). So that both "class Token AAA" and "constructor AAA" is returned.

Later, "class Token AAA" is removed from the result, so we go to the correct declaration of the constructor.
I now understand. I was confused about the sentence in bold because in my initial tests, the constructors were actually added to the result even without your AddConstructors() patch (and I got the "Multiple matches" dialog). But that was only because I was already inside the class scope when performing the "go to impl". In other cases (as in your example code) the AddConstructors() is indeed required.

I'm not sure my patch is going to handle "go to implementation xxxxx", I just think it solve some "go to declaration xxxx" issue.
I tested your patch and can confirm it handles both cases as expected.

So I'm ok about committing your entire patch. :)
« Last Edit: March 07, 2014, 04:10:08 am by Huki »