From e637de145af8aff3cbb0f0761928c6496eb9bed2 Mon Sep 17 00:00:00 2001 From: Jacob Dufault Date: Sun, 19 Feb 2017 01:05:57 -0800 Subject: [PATCH] better-return-type-location --- main.cpp | 32 ++++++++++++--------- tests/usage/type_usage_on_return_type.cc | 36 +++++++++++++++++++++--- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/main.cpp b/main.cpp index 254bd55f..2a44649b 100644 --- a/main.cpp +++ b/main.cpp @@ -639,7 +639,7 @@ clang::VisiterResult VarDeclVisitor(clang::Cursor cursor, clang::Cursor parent, case CXCursor_UnaryOperator: return clang::VisiterResult::Continue; /* - + case CXCursor_CallExpr: // TODO: Add a test for parameters inside the call? We should probably recurse. InsertReference(param->db, param->func_id, cursor); @@ -661,7 +661,7 @@ void HandleVarDecl(ParsingDatabase* db, NamespaceStack* ns, clang::Cursor var, s // Add usage to types. VarDeclVisitorParam varDeclVisitorParam(db, func_id); var.VisitChildren(&VarDeclVisitor, &varDeclVisitorParam); - + if (!declare_variable) return; @@ -741,14 +741,23 @@ struct FuncDefinitionParam { NamespaceStack* ns; FuncId func_id; bool is_definition; + bool has_return_type; - FuncDefinitionParam(ParsingDatabase* db, NamespaceStack* ns, FuncId func_id, bool is_definition) - : db(db), ns(ns), func_id(func_id), is_definition(is_definition) {} + FuncDefinitionParam(ParsingDatabase* db, NamespaceStack* ns, FuncId func_id, bool is_definition, bool has_return_type) + : db(db), ns(ns), func_id(func_id), is_definition(is_definition), has_return_type(has_return_type) {} }; clang::VisiterResult VisitFuncDefinition(clang::Cursor cursor, clang::Cursor parent, FuncDefinitionParam* param) { + if (param->has_return_type) { + // Foo* Foo::Bar() {} will have two TypeRef nodes. + assert(cursor.get_kind() == CXCursor_TypeRef); + InsertTypeUsageAtLocation(param->db, cursor.get_referenced().get_type(), cursor.get_source_location()); + param->has_return_type = false; + } + //std::cout << "VistFuncDefinition got " << cursor.ToString() << std::endl; switch (cursor.get_kind()) { + // TODO: Maybe we should default to recurse? /* case CXCursor_CompoundStmt: @@ -814,13 +823,6 @@ void HandleFunc(ParsingDatabase* db, NamespaceStack* ns, clang::Cursor func, std func_def->declaring_type = declaring_type; } - // Insert return type usage here instead of in the visitor. The only way to - // do it in the visitor is to search for CXCursor_TypeRef, which does not - // necessarily refer to the return type. - // TODO: Is that the case? What about a top-level visitor? Would return type - // location be better? - InsertTypeUsageAtLocation(db, func.get_type().get_return_type(), func.get_source_location()); - // Don't process definition/body for declarations. if (!func.is_definition()) { func_def->declaration = func.get_source_location(); @@ -844,7 +846,11 @@ void HandleFunc(ParsingDatabase* db, NamespaceStack* ns, clang::Cursor func, std if (func.is_definition()) func_def->definition = func.get_source_location(); - FuncDefinitionParam funcDefinitionParam(db, &NamespaceStack::kEmpty, func_id, func.is_definition()); + // Ignore any fundamental types for return. Note that void is a fundamental + // type. + bool has_return_type = !func.get_type().get_return_type().is_fundamental(); + + FuncDefinitionParam funcDefinitionParam(db, &NamespaceStack::kEmpty, func_id, func.is_definition(), has_return_type); func.VisitChildren(&VisitFuncDefinition, &funcDefinitionParam); } @@ -1119,7 +1125,7 @@ int main(int argc, char** argv) { // TODO: Fix all existing tests. //if (path != "tests/constructors/constructor.cc") continue; //if (path != "tests/usage/func_usage_addr_func.cc") continue; - //if (path != "tests/vars/class_static_member.cc") continue; + //if (path != "tests/usage/type_usage_on_return_type.cc") continue; // Parse expected output from the test, parse it into JSON document. std::string expected_output; diff --git a/tests/usage/type_usage_on_return_type.cc b/tests/usage/type_usage_on_return_type.cc index 28961a3e..753d1936 100644 --- a/tests/usage/type_usage_on_return_type.cc +++ b/tests/usage/type_usage_on_return_type.cc @@ -4,10 +4,15 @@ Type* foo(); Type* foo(); Type* foo() {} -/* -// TODO: We should try to get the right location for type uses so it points to -// the return type and not the function name. +class Foo { + Type* Get(int); + void Empty(); +}; +Type* Foo::Get(int) {} +void Foo::Empty() {} + +/* OUTPUT: { "types": [{ @@ -16,7 +21,14 @@ OUTPUT: "short_name": "Type", "qualified_name": "Type", "declaration": "tests/usage/type_usage_on_return_type.cc:1:8", - "uses": ["tests/usage/type_usage_on_return_type.cc:3:7", "tests/usage/type_usage_on_return_type.cc:4:7", "tests/usage/type_usage_on_return_type.cc:5:7"] + "uses": ["tests/usage/type_usage_on_return_type.cc:3:1", "tests/usage/type_usage_on_return_type.cc:4:1", "tests/usage/type_usage_on_return_type.cc:5:1", "tests/usage/type_usage_on_return_type.cc:8:3", "tests/usage/type_usage_on_return_type.cc:12:1"] + }, { + "id": 1, + "usr": "c:@S@Foo", + "short_name": "Foo", + "qualified_name": "Foo", + "definition": "tests/usage/type_usage_on_return_type.cc:7:7", + "funcs": [1, 2] }], "functions": [{ "id": 0, @@ -25,6 +37,22 @@ OUTPUT: "qualified_name": "foo", "declaration": "tests/usage/type_usage_on_return_type.cc:4:7", "definition": "tests/usage/type_usage_on_return_type.cc:5:7" + }, { + "id": 1, + "usr": "c:@S@Foo@F@Get#I#", + "short_name": "Get", + "qualified_name": "Foo::Get", + "declaration": "tests/usage/type_usage_on_return_type.cc:8:9", + "definition": "tests/usage/type_usage_on_return_type.cc:12:12", + "declaring_type": 1 + }, { + "id": 2, + "usr": "c:@S@Foo@F@Empty#", + "short_name": "Empty", + "qualified_name": "Foo::Empty", + "declaration": "tests/usage/type_usage_on_return_type.cc:9:8", + "definition": "tests/usage/type_usage_on_return_type.cc:13:11", + "declaring_type": 1 }], "variables": [] }