From 11af3986baec88fc337a5fc61cb6242902d7b272 Mon Sep 17 00:00:00 2001 From: Jacob Dufault Date: Thu, 15 Jun 2017 23:43:02 -0700 Subject: [PATCH] Better symbol resolution (ie, goto definition) for macro arguments. --- src/clang_utils.cc | 1 + src/lex_utils.cc | 19 +++++++++++-- src/query_utils.cc | 29 ++++++++++++++++---- src/working_files.cc | 3 ++- tests/macros/complex.cc | 59 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 8 deletions(-) create mode 100644 tests/macros/complex.cc diff --git a/src/clang_utils.cc b/src/clang_utils.cc index c6b15767..70aac896 100644 --- a/src/clang_utils.cc +++ b/src/clang_utils.cc @@ -39,6 +39,7 @@ optional BuildAndDisposeDiagnostic(CXDiagnostic diagnostic) { lsDiagnostic ls_diagnostic; + // TODO: consider using clang_getDiagnosticRange // TODO: ls_diagnostic.range is lsRange, we have Range. We should only be // storing Range types when inside the indexer so that index <-> buffer // remapping logic is applied. diff --git a/src/lex_utils.cc b/src/lex_utils.cc index 13c26e8a..112743d1 100644 --- a/src/lex_utils.cc +++ b/src/lex_utils.cc @@ -2,17 +2,22 @@ #include +#include + int GetOffsetForPosition(lsPosition position, const std::string& content) { + if (content.empty()) + return 0; + int offset = 0; int remaining_lines = position.line; - while (remaining_lines > 0) { + while (remaining_lines > 0 && offset < content.size()) { if (content[offset] == '\n') --remaining_lines; ++offset; } - return offset + position.character; + return std::min(offset + position.character, content.size() - 1); } lsPosition CharPos(const std::string& search, char character, int character_offset) { @@ -204,6 +209,16 @@ bool SubstringMatch(const std::string& search, const std::string& content) { return false; } +TEST_SUITE("Offset"); + +TEST_CASE("over range") { + std::string content = "foo"; + int offset = GetOffsetForPosition(lsPosition(10, 10), content); + REQUIRE(offset < content.size()); +} + +TEST_SUITE_END(); + TEST_SUITE("Substring"); TEST_CASE("match") { diff --git a/src/query_utils.cc b/src/query_utils.cc index 4ae0d9f7..cd7c3bb8 100644 --- a/src/query_utils.cc +++ b/src/query_utils.cc @@ -1,5 +1,16 @@ #include "query_utils.h" +namespace { + +// Computes roughly how long |range| is. +int ComputeRangeSize(const Range& range) { + if (range.start.line != range.end.line) + return INT_MAX; + return range.end.column - range.start.column; +} + +} // namespace + optional GetDefinitionSpellingOfSymbol(QueryDatabase* db, const QueryTypeId& id) { optional& type = db->types[id.id]; if (type) @@ -550,12 +561,20 @@ std::vector FindSymbolsAtLocation(WorkingFile* working_file, QueryFil symbols.push_back(ref); } - // Order function symbols first. This makes goto definition work better when - // used on a constructor. + // Order shorter ranges first, since they are more detailed/precise. This is + // important for macros which generate code so that we can resolving the + // macro argument takes priority over the entire macro body. + // + // Order functions before other types, which makes goto definition work + // better on constructors. std::sort(symbols.begin(), symbols.end(), [](const SymbolRef& a, const SymbolRef& b) { - if (a.idx.kind != b.idx.kind && a.idx.kind == SymbolKind::Func) - return 1; - return 0; + int a_size = ComputeRangeSize(a.loc.range); + int b_size = ComputeRangeSize(b.loc.range); + + if (a_size == b_size) + return a.idx.kind != b.idx.kind && a.idx.kind == SymbolKind::Func; + + return a_size < b_size; }); return symbols; diff --git a/src/working_files.cc b/src/working_files.cc index 38ee2523..33589f9c 100644 --- a/src/working_files.cc +++ b/src/working_files.cc @@ -404,7 +404,8 @@ TEST_CASE("auto-insert )") { } TEST_CASE("existing completion") { - WorkingFile f("foo.cc", "zzz.asdf"); + // TODO: remove trailing space in zz.asdf. Lexing doesn't work correctly if done at the end of input. + WorkingFile f("foo.cc", "zzz.asdf "); bool is_global_completion; std::string existing_completion; diff --git a/tests/macros/complex.cc b/tests/macros/complex.cc new file mode 100644 index 00000000..8d214373 --- /dev/null +++ b/tests/macros/complex.cc @@ -0,0 +1,59 @@ +#define FOO(aaa, bbb) \ + int a();\ + int a() { return aaa + bbb; } + + +int make1() { + return 3; +} +const int make2 = 5; + + +FOO(make1(), make2); + +/* +OUTPUT: +{ + "funcs": [{ + "id": 0, + "usr": "c:@F@make1#", + "short_name": "make1", + "detailed_name": "int make1()", + "definition_spelling": "6:5-6:10", + "definition_extent": "6:1-8:2", + "callers": ["1@12:5-12:10"] + }, { + "id": 1, + "usr": "c:@F@a#", + "short_name": "a", + "detailed_name": "int a()", + "declarations": [{ + "spelling": "12:1-12:20", + "extent": "12:1-12:20", + "content": "int a();\n int a() { return aaa + bbb; }\n\n\n int make1() {\n return 3;\n }\n const int make2 = 5;\n\n\n FOO(make1(), make2)" + }], + "definition_spelling": "12:1-12:20", + "definition_extent": "12:1-12:20", + "callees": ["0@12:5-12:10"] + }], + "vars": [{ + "id": 0, + "usr": "c:complex.cc@make2", + "short_name": "make2", + "detailed_name": "const int make2", + "definition_spelling": "9:11-9:16", + "definition_extent": "9:1-9:20", + "is_local": false, + "uses": ["9:11-9:16", "12:14-12:19"] + }, { + "id": 1, + "usr": "c:complex.cc@8@macro@FOO", + "short_name": "FOO", + "detailed_name": "FOO", + "definition_spelling": "1:9-1:12", + "definition_extent": "1:9-3:32", + "is_local": false, + "uses": ["1:9-1:12", "12:1-12:4"] + }] +} +*/ \ No newline at end of file