Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++ parser does not tag member variable if type is template with forward declared class #2867

Closed
drichardson opened this issue Feb 14, 2021 · 6 comments · Fixed by #2869 or #3055
Closed
Assignees

Comments

@drichardson
Copy link
Contributor

drichardson commented Feb 14, 2021

The name of the parser: C++

I wrote a unit test that I believe should pass in this branch:

$ python3 misc/units.py run Units/parser-cxx.r --units template-member-forward-declaration

Category: ROOT
------------------------------------------------------------
Testing template-member-forward-declaration as C++          failed (unexpected output)

Summary (see CMDLINE.tmp to reproduce without test harness)
------------------------------------------------------------
  #passed:                                0
  #FIXED:                                 0
  #FAILED (broken args.ctags?):           0
  #FAILED (unexpected-exit-status):       0
  #FAILED (unexpected-output):            1
        template-member-forward-declaration
  #skipped (features):                    0
  #skipped (languages):                   0
  #skipped (infinite-loop):               0
  #known-bugs:                            0

The content of input file:

template <class Class>
class A
{
public:
        A() { p = new Class(); }
        Class *p;
};

class B {
public:
        // forward declaration okay because only used as pointer in Template
        A<class C> a_c_forward;
};

Running ctags with the options --sort=no --kinds-C++=m produces tags output that is missing an entry for a_c_forward.

p       input.cpp       /^      Class *p;$/;"   m       class:A typeref:typename:Class *        file:

The tags output you expect:

p       input.cpp       /^      Class *p;$/;"   m       class:A typeref:typename:Class *        file:
a_c_forward     input.cpp       /^      A<class C> a_c_forward;$/;"     m       class:B typeref:typename:A<class C>     file:

The version of ctags:

Universal Ctags 5.9.0(3088bfc1), Copyright (C) 2015 Universal Ctags Team
Universal Ctags is derived from Exuberant Ctags.
Exuberant Ctags 5.8, Copyright (C) 1996-2009 Darren Hiebert
  Compiled: Feb 14 2021, 14:45:13
  URL: https://ctags.io/
  Optional compiled features: +wildcards, +regex, +iconv, +debug, +option-directory, +packcc

How do you get ctags binary: Built locally.

Additional Info

Here is a ctags trace. You can see that the class token is what is causing the parser to not treat the reset of the line as a variable declaration.

@drichardson
Copy link
Contributor Author

It looks like this is the case that causes the parser to try to handle it like a class instead of a member variable.

Based on the name and the switch statement cases around it, I imagine this happens for unions and structs as well.

@masatake
Copy link
Member

masatake commented Feb 15, 2021

Thank you for reporting.

It seems that our maintainer of C/C++ parsers is busy now.
Here is my idea for fixing.

diff --git a/parsers/cxx/cxx_parser_block.c b/parsers/cxx/cxx_parser_block.c
index 0d6ea947d..8abcadb5a 100644
--- a/parsers/cxx/cxx_parser_block.c
+++ b/parsers/cxx/cxx_parser_block.c
@@ -262,6 +262,7 @@ static bool cxxParserParseBlockInternal(bool bExpectClosingBracket)
                cppBeginStatement();
        }
 
+       int in_triangle = 0;
        for(;;)
        {
                if(!cxxParserParseNextToken())
@@ -392,7 +393,8 @@ process_token:
                                                }
                                        break;
                                        case CXXKeywordCLASS:
-                                               if(!cxxParserParseClassStructOrUnion(CXXKeywordCLASS,CXXTagCPPKindCLASS,CXXScopeTypeClass))
+                                               if(in_triangle == 0 &&
+                                                  !cxxParserParseClassStructOrUnion(CXXKeywordCLASS,CXXTagCPPKindCLASS,CXXScopeTypeClass))
                                                {
                                                        CXX_DEBUG_LEAVE_TEXT("Failed to parse class/struct/union");
                                                        return false;
@@ -744,6 +746,12 @@ process_token:
                                else if (cxxScopeGetType() == CXXScopeTypeClass)
                                        cxxSubparserUnknownIdentifierInClassNotify(g_cxx.pToken);
                        break;
+                       case CXXTokenTypeSmallerThanSign:
+                               in_triangle++;
+                               break;
+                       case CXXTokenTypeGreaterThanSign:
+                               in_triangle--;
+                               break;
                        default:
                                // something else we didn't handle
                        break;

@masatake
Copy link
Member

I must consider not only class but also struct and, union.

diff --git a/parsers/cxx/cxx_parser_block.c b/parsers/cxx/cxx_parser_block.c
index 0d6ea947d..80f16229c 100644
--- a/parsers/cxx/cxx_parser_block.c
+++ b/parsers/cxx/cxx_parser_block.c
@@ -262,6 +262,7 @@ static bool cxxParserParseBlockInternal(bool bExpectClosingBracket)
 		cppBeginStatement();
 	}
 
+	int in_triangle = 0;
 	for(;;)
 	{
 		if(!cxxParserParseNextToken())
@@ -392,21 +393,24 @@ process_token:
 						}
 					break;
 					case CXXKeywordCLASS:
-						if(!cxxParserParseClassStructOrUnion(CXXKeywordCLASS,CXXTagCPPKindCLASS,CXXScopeTypeClass))
+						if(in_triangle == 0 &&
+						   !cxxParserParseClassStructOrUnion(CXXKeywordCLASS,CXXTagCPPKindCLASS,CXXScopeTypeClass))
 						{
 							CXX_DEBUG_LEAVE_TEXT("Failed to parse class/struct/union");
 							return false;
 						}
 					break;
 					case CXXKeywordSTRUCT:
-						if(!cxxParserParseClassStructOrUnion(CXXKeywordSTRUCT,CXXTagKindSTRUCT,CXXScopeTypeStruct))
+						if(in_triangle == 0 &&
+						   !cxxParserParseClassStructOrUnion(CXXKeywordSTRUCT,CXXTagKindSTRUCT,CXXScopeTypeStruct))
 						{
 							CXX_DEBUG_LEAVE_TEXT("Failed to parse class/struct/union");
 							return false;
 						}
 					break;
 					case CXXKeywordUNION:
-						if(!cxxParserParseClassStructOrUnion(CXXKeywordUNION,CXXTagKindUNION,CXXScopeTypeUnion))
+						if(in_triangle == 0 &&
+						   !cxxParserParseClassStructOrUnion(CXXKeywordUNION,CXXTagKindUNION,CXXScopeTypeUnion))
 						{
 							CXX_DEBUG_LEAVE_TEXT("Failed to parse class/struct/union");
 							return false;
@@ -744,6 +748,12 @@ process_token:
 				else if (cxxScopeGetType() == CXXScopeTypeClass)
 					cxxSubparserUnknownIdentifierInClassNotify(g_cxx.pToken);
 			break;
+			case CXXTokenTypeSmallerThanSign:
+				in_triangle++;
+				break;
+			case CXXTokenTypeGreaterThanSign:
+				in_triangle--;
+				break;
 			default:
 				// something else we didn't handle
 			break;

@drichardson
Copy link
Contributor Author

drichardson commented Feb 15, 2021

@masatake

Thanks! I just tried your diff and it passes make check in my branch (including the test I wrote).

Do you want to make a PR for this, or do you want me to add your diff to the branch I linked to above?

@drichardson
Copy link
Contributor Author

@masatake I went ahead and made a PR #2869. I committed the diff you pasted with you listed as the author.

@masatake
Copy link
Member

masatake commented Feb 16, 2021

@masatake I went ahead and made a PR #2869. I committed the diff you pasted with you listed as the author.

Thank you!

I asked you to squash the commits. In that case, you can remove my name from the unified commit if you need.

drichardson added a commit to drichardson/ctags that referenced this issue Feb 16, 2021
Fixes universal-ctags#2867.

Co-authored-by: Masatake YAMATO <yamato@redhat.com>
drichardson added a commit to drichardson/ctags that referenced this issue Feb 16, 2021
drichardson added a commit to drichardson/ctags that referenced this issue Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants