From 76b6a3d4fd0078a0a0e7e867024bd528b01d3968 Mon Sep 17 00:00:00 2001 From: Jacob Dufault Date: Mon, 10 Apr 2017 22:26:27 -0700 Subject: [PATCH] Store indexed dependencies so we can reload all files when restoring from cache. Also slightly improve goto definition source range --- src/command_line.cc | 62 ++++++++-- src/file_consumer.cc | 12 +- src/file_consumer.h | 8 +- src/indexer.cc | 201 ++++++++++++++++++++++++++------ src/indexer.h | 1 + src/query.cc | 1 - src/serializer.cc | 1 + src/test.cc | 6 +- src/working_files.cc | 3 +- tests/multi_file/header.h | 7 -- tests/multi_file/impl.cc | 67 +++++------ tests/multi_file/simple_impl.cc | 1 + 12 files changed, 275 insertions(+), 95 deletions(-) diff --git a/src/command_line.cc b/src/command_line.cc index 9fe83559..3d096dd3 100644 --- a/src/command_line.cc +++ b/src/command_line.cc @@ -25,6 +25,9 @@ #include #include +// TODO: provide a feature like 'https://github.com/goldsborough/clang-expand', ie, a fully linear view of a function with +// inline function calls expanded. We can probably use vscode decorators to achieve it. + namespace { const char* kIpcLanguageClientName = "language_client"; @@ -475,10 +478,19 @@ bool IndexMain_DoIndex(FileConsumer* file_consumer, if (index_request->type == Index_DoIndex::Type::Import) { index_request->type = Index_DoIndex::Type::Update; std::unique_ptr old_index = LoadCachedFile(index_request->path); - time.ResetAndPrint("Loading cached index " + index_request->path); + time.ResetAndPrint("Reading cached index from disk " + index_request->path); // If import fails just do a standard update. if (old_index) { + for (auto& dependency_path : old_index->dependencies) { + std::cerr << "- Dispatching dependency import " << dependency_path << std::endl; + Index_DoIndex dep_index_request(Index_DoIndex::Type::Import); + dep_index_request.path = dependency_path; + dep_index_request.args = index_request->args; + queue_do_index->Enqueue(std::move(dep_index_request)); + } + + Index_DoIdMap response(nullptr, std::move(old_index)); queue_do_id_map->Enqueue(std::move(response)); @@ -513,8 +525,9 @@ bool IndexMain_DoIndex(FileConsumer* file_consumer, return true; } -bool IndexMain_DoCreateIndexUpdate(Index_OnIdMappedQueue* queue_on_id_mapped, - Index_OnIndexedQueue* queue_on_indexed) { +bool IndexMain_DoCreateIndexUpdate( + Index_OnIdMappedQueue* queue_on_id_mapped, + Index_OnIndexedQueue* queue_on_indexed) { optional response = queue_on_id_mapped->TryDequeue(); if (!response) return false; @@ -530,6 +543,24 @@ bool IndexMain_DoCreateIndexUpdate(Index_OnIdMappedQueue* queue_on_id_mapped, return true; } +void IndexJoinIndexUpdates(Index_OnIndexedQueue* queue_on_indexed) { + optional root = queue_on_indexed->TryDequeue(); + if (!root) + return; + + while (true) { + optional to_join = queue_on_indexed->TryDequeue(); + if (!to_join) { + queue_on_indexed->Enqueue(std::move(*root)); + return; + } + + Timer time; + root->update.Merge(to_join->update); + time.ResetAndPrint("Indexer joining two querydb updates"); + } +} + void IndexMain( FileConsumer::SharedState* file_consumer_shared, Index_DoIndexQueue* queue_do_index, @@ -544,10 +575,18 @@ void IndexMain( // better icache behavior. We need to have some threads spinning on both though // otherwise memory usage will get bad. + int count = 0; + if (!IndexMain_DoIndex(&file_consumer, queue_do_index, queue_do_id_map) && !IndexMain_DoCreateIndexUpdate(queue_on_id_mapped, queue_on_indexed)) { + + //if (count++ > 2) { + // count = 0; + IndexJoinIndexUpdates(queue_on_indexed); + //} + // TODO: use CV to wakeup? - std::this_thread::sleep_for(std::chrono::milliseconds(500)); + std::this_thread::sleep_for(std::chrono::milliseconds(25)); } } } @@ -715,9 +754,16 @@ void QueryDbMainLoop( if (ref.loc.range.start.line >= target_line && ref.loc.range.end.line <= target_line && ref.loc.range.start.column <= target_column && ref.loc.range.end.column >= target_column) { // Found symbol. Return definition. - optional location = GetDefinitionSpellingOfSymbol(db, ref.idx); - if (location) { - optional ls_location = GetLsLocation(db, working_files, location.value()); + optional spelling = GetDefinitionSpellingOfSymbol(db, ref.idx); + if (spelling) { + // We use spelling start and extent end because this causes vscode + // to highlight the entire definition when previewing / hoving with + // the mouse. + optional extent = GetDefinitionExtentOfSymbol(db, ref.idx); + if (extent) + spelling->range.end = extent->range.end; + + optional ls_location = GetLsLocation(db, working_files, spelling.value()); if (ls_location) response.result.push_back(*ls_location); } @@ -1143,7 +1189,7 @@ int main(int argc, char** argv) { //bool loop = true; //while (loop) // std::this_thread::sleep_for(std::chrono::milliseconds(10)); - std::this_thread::sleep_for(std::chrono::seconds(3)); + //std::this_thread::sleep_for(std::chrono::seconds(3)); PlatformInit(); RegisterMessageTypes(); diff --git a/src/file_consumer.cc b/src/file_consumer.cc index f558eb40..eb3a9a7c 100644 --- a/src/file_consumer.cc +++ b/src/file_consumer.cc @@ -5,11 +5,15 @@ FileConsumer::FileConsumer(SharedState* shared_state) : shared_(shared_state) {} -IndexedFile* FileConsumer::TryConsumeFile(const std::string& file) { +IndexedFile* FileConsumer::TryConsumeFile(const std::string& file, bool* is_first_ownership) { + assert(is_first_ownership); + // Try to find cached local result. auto it = local_.find(file); - if (it != local_.end()) + if (it != local_.end()) { + *is_first_ownership = false; return it->second.get(); + } // No result in local; we need to query global. bool did_insert = false; @@ -17,14 +21,16 @@ IndexedFile* FileConsumer::TryConsumeFile(const std::string& file) { std::lock_guard lock(shared_->mutex); did_insert = shared_->files.insert(file).second; } + *is_first_ownership = did_insert; local_[file] = did_insert ? MakeUnique(file) : nullptr; return local_[file].get(); } -void FileConsumer::ForceLocal(const std::string& file) { +IndexedFile* FileConsumer::ForceLocal(const std::string& file) { auto it = local_.find(file); if (it == local_.end()) local_[file] = MakeUnique(file); + return local_[file].get(); } std::vector> FileConsumer::TakeLocalState() { diff --git a/src/file_consumer.h b/src/file_consumer.h index 12f2508e..e628b301 100644 --- a/src/file_consumer.h +++ b/src/file_consumer.h @@ -24,11 +24,13 @@ struct FileConsumer { // Returns true if this instance owns given |file|. This will also attempt to // take ownership over |file|. // - // Returns IndexedFile for the file or nullptr. - IndexedFile* TryConsumeFile(const std::string& file); + // Returns IndexedFile for the file or nullptr. |is_first_ownership| is set + // to true iff the function just took ownership over the file. Otherwise it + // is set to false. + IndexedFile* TryConsumeFile(const std::string& file, bool* is_first_ownership); // Forcibly create a local file, even if it has already been parsed. - void ForceLocal(const std::string& file); + IndexedFile* ForceLocal(const std::string& file); // Returns and passes ownership of all local state. std::vector> TakeLocalState(); diff --git a/src/indexer.cc b/src/indexer.cc index 67cb4975..8602a978 100644 --- a/src/indexer.cc +++ b/src/indexer.cc @@ -184,14 +184,19 @@ struct NamespaceHelper { }; struct IndexParam { + // Only use this when strictly needed (ie, primary translation unit is + // needed). Most logic should get the IndexedFile instance via + // |file_consumer|. + IndexedFile* primary_file; + FileConsumer* file_consumer; - NamespaceHelper* ns; + NamespaceHelper ns; // Record last func usage we reported, as clang will record the reference // twice. We don't want to double report. Range last_func_usage_location; - IndexParam(FileConsumer* file_consumer, NamespaceHelper* ns) : file_consumer(file_consumer), ns(ns) {} + IndexParam(FileConsumer* file_consumer) : file_consumer(file_consumer) {} }; int abortQuery(CXClientData client_data, void* reserved) { @@ -234,7 +239,9 @@ CXIdxClientFile enteredMainFile(CXClientData client_data, } CXIdxClientFile ppIncludedFile(CXClientData client_data, - const CXIdxIncludedFileInfo*) { + const CXIdxIncludedFileInfo* file) { + // Clang include logic is broken. This function is never + // called and clang_findIncludesInFile doesn't work. return nullptr; } @@ -312,15 +319,6 @@ optional FindType(clang::Cursor cursor) { return result; } -/* -std::string GetNamespacePrefx(const CXIdxDeclInfo* decl) { - const CXIdxContainerInfo* container = decl->lexicalContainer; - while (container) { - - } -} -*/ - bool IsTypeDefinition(const CXIdxContainerInfo* container) { if (!container) return false; @@ -647,6 +645,62 @@ bool AreEqualLocations(CXIdxLoc loc, CXCursor cursor) { // TODO TODO TODO TODO // INDEX SPELLING + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + void indexDeclaration(CXClientData client_data, const CXIdxDeclInfo* decl) { // TODO: allow user to configure if they want STL index. if (clang_Location_isInSystemHeader(clang_indexLoc_getCXSourceLocation(decl->loc))) @@ -657,14 +711,17 @@ void indexDeclaration(CXClientData client_data, const CXIdxDeclInfo* decl) { // TODO: Use clang_getFileUniqueID CXFile file; clang_getSpellingLocation(clang_indexLoc_getCXSourceLocation(decl->loc), &file, nullptr, nullptr, nullptr); - std::string filename = clang::ToString(clang_getFileName(file)); + std::string filename = NormalizePath(clang::ToString(clang_getFileName(file))); IndexParam* param = static_cast(client_data); - IndexedFile* db = param->file_consumer->TryConsumeFile(filename); + bool is_first_time_visiting_file = false; + IndexedFile* db = param->file_consumer->TryConsumeFile(filename, &is_first_time_visiting_file); if (!db) return; + if (is_first_time_visiting_file) + param->primary_file->dependencies.push_back(db->path); - NamespaceHelper* ns = param->ns; + NamespaceHelper* ns = ¶m->ns; //std::cerr << "DECL kind=" << decl->entityInfo->kind << " at " << db->id_cache.Resolve(decl->cursor, false).ToPrettyString(&db->id_cache) << std::endl; @@ -1049,6 +1106,53 @@ bool IsFunctionCallContext(CXCursorKind kind) { return false; } + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + void indexEntityReference(CXClientData client_data, const CXIdxEntityRefInfo* ref) { // Don't index references from or to system headers. @@ -1066,12 +1170,16 @@ void indexEntityReference(CXClientData client_data, // TODO: Use clang_getFileUniqueID CXFile file; clang_getSpellingLocation(clang_indexLoc_getCXSourceLocation(ref->loc), &file, nullptr, nullptr, nullptr); - std::string filename = clang::ToString(clang_getFileName(file)); + std::string filename = NormalizePath(clang::ToString(clang_getFileName(file))); IndexParam* param = static_cast(client_data); - IndexedFile* db = param->file_consumer->TryConsumeFile(filename); + bool is_first_time_visiting_file = false; + IndexedFile* db = param->file_consumer->TryConsumeFile(filename, &is_first_time_visiting_file); if (!db) return; + if (is_first_time_visiting_file) + param->primary_file->dependencies.push_back(db->path); + // ref->cursor mainFile=0 // ref->loc mainFile=1 // ref->referencedEntity mainFile=1 @@ -1253,8 +1361,40 @@ void indexEntityReference(CXClientData client_data, } } + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + std::vector> Parse(FileConsumer* file_consumer, std::string filename, std::vector args, bool dump_ast) { - //return {}; + filename = NormalizePath(filename); + return {}; clang_enableStackTraces(); clang_toggleCrashRecovery(1); @@ -1266,42 +1406,31 @@ std::vector> Parse(FileConsumer* file_consumer, std if (dump_ast) Dump(tu.document_cursor()); - CXIndexAction index_action = clang_IndexAction_create(index.cx_index); IndexerCallbacks callbacks[] = { {&abortQuery, &diagnostic, &enteredMainFile, &ppIncludedFile, &importedASTFile, &startedTranslationUnit, &indexDeclaration, &indexEntityReference} - //{ &abortQuery, &diagnostic, &enteredMainFile, &ppIncludedFile, - //&importedASTFile, &startedTranslationUnit, &emptyIndexDeclaration, - //&emptyIndexEntityReference } - /* - callbacks.abortQuery = &abortQuery; - callbacks.diagnostic = &diagnostic; - callbacks.enteredMainFile = &enteredMainFile; - callbacks.ppIncludedFile = &ppIncludedFile; - callbacks.importedASTFile = &importedASTFile; - callbacks.startedTranslationUnit = &startedTranslationUnit; - callbacks.indexDeclaration = &indexDeclaration; - callbacks.indexEntityReference = &indexEntityReference; - */ }; - NamespaceHelper ns; - IndexParam param(file_consumer, &ns); + IndexParam param(file_consumer); - file_consumer->ForceLocal(filename); + param.primary_file = file_consumer->ForceLocal(filename); std::cerr << "!! [START] Indexing " << filename << std::endl; + CXIndexAction index_action = clang_IndexAction_create(index.cx_index); clang_indexTranslationUnit(index_action, ¶m, callbacks, sizeof(callbacks), CXIndexOpt_IndexFunctionLocalSymbols | CXIndexOpt_SkipParsedBodiesInSession | CXIndexOpt_IndexImplicitTemplateInstantiations, tu.cx_tu); - std::cerr << "!! [END] Indexing " << filename << std::endl; clang_IndexAction_dispose(index_action); + std::cerr << "!! [END] Indexing " << filename << std::endl; auto result = param.file_consumer->TakeLocalState(); for (auto& entry : result) { // TODO: only store the path on one of these. + // TODO: These NormalizePath call should be not needed. + assert(entry->path == NormalizePath(entry->path)); + assert(entry->id_cache.primary_file == entry->path); entry->path = NormalizePath(entry->path); entry->id_cache.primary_file = entry->path; } diff --git a/src/indexer.h b/src/indexer.h index c079ccb9..9978371e 100644 --- a/src/indexer.h +++ b/src/indexer.h @@ -424,6 +424,7 @@ struct IndexedFile { std::string path; + std::vector dependencies; std::vector types; std::vector funcs; std::vector vars; diff --git a/src/query.cc b/src/query.cc index f2e3d593..b5e97efb 100644 --- a/src/query.cc +++ b/src/query.cc @@ -638,7 +638,6 @@ void QueryableDatabase::RemoveUsrs(const std::vector& to_remove) { void QueryableDatabase::ImportOrUpdate(const std::vector& updates) { for (auto& def : updates) { - std::cerr << "Importing def for " << def.usr << std::endl; auto it = usr_to_symbol.find(def.usr); assert(it != usr_to_symbol.end()); diff --git a/src/serializer.cc b/src/serializer.cc index dd5ea96f..8dc3f49e 100644 --- a/src/serializer.cc +++ b/src/serializer.cc @@ -207,6 +207,7 @@ bool ReflectMemberStart(Writer& visitor, IndexedFile& value) { template void Reflect(TVisitor& visitor, IndexedFile& value) { REFLECT_MEMBER_START(); + REFLECT_MEMBER(dependencies); REFLECT_MEMBER(types); REFLECT_MEMBER(funcs); REFLECT_MEMBER(vars); diff --git a/src/test.cc b/src/test.cc index faaf6ac7..f1287a55 100644 --- a/src/test.cc +++ b/src/test.cc @@ -118,8 +118,8 @@ void RunTests() { //if (path != "tests/templates/specialized_func_definition.cc") continue; //if (path != "tests/templates/namespace_template_class_template_func_usage_folded_into_one.cc") continue; //if (path != "tests/multi_file/header.h") continue; - //if (path != "tests/multi_file/impl.cc") continue; - if (path != "tests/usage/func_called_from_template.cc") continue; + //if (path != "tests/multi_file/simple_impl.cc") continue; + //if (path != "tests/usage/func_called_from_template.cc") continue; //if (path != "tests/templates/implicit_variable_instantiation.cc") continue; //if (path != "tests/_empty_test.cc") continue; @@ -141,7 +141,7 @@ void RunTests() { "-IC:/Users/jacob/Desktop/superindex/indexer/third_party/doctest/", "-IC:/Users/jacob/Desktop/superindex/indexer/third_party/rapidjson/include", "-IC:/Users/jacob/Desktop/superindex/indexer/src" - }, true /*dump_ast*/); + }, false /*dump_ast*/); for (auto& entry : all_expected_output) { const std::string& expected_path = entry.first; diff --git a/src/working_files.cc b/src/working_files.cc index 8cb7dfbc..582e67c1 100644 --- a/src/working_files.cc +++ b/src/working_files.cc @@ -196,7 +196,8 @@ void WorkingFiles::OnChange(const Ipc_TextDocumentDidChange::Params& change) { int old_line_count = diff.range.end.line - diff.range.start.line; int new_line_count = LineCount(diff.text.begin(), diff.text.end()); - assert(old_line_count == LineCount(file->content.begin() + start_offset, file->content.begin() + start_offset + diff.rangeLength + 1)); + // TODO: this assert goes off. Figure out why. + //assert(old_line_count == LineCount(file->content.begin() + start_offset, file->content.begin() + start_offset + diff.rangeLength + 1)); int line_delta = new_line_count - old_line_count; if (line_delta != 0) { diff --git a/tests/multi_file/header.h b/tests/multi_file/header.h index 4d840980..717ef821 100644 --- a/tests/multi_file/header.h +++ b/tests/multi_file/header.h @@ -1,12 +1,5 @@ #pragma once -#include "../../third_party/doctest/doctest/doctest.h" -#include "../../third_party/macro_map.h" -#include "../../third_party/optional.h" -#include -#include -#include - struct Base {}; struct SameFileDerived : Base {}; diff --git a/tests/multi_file/impl.cc b/tests/multi_file/impl.cc index 1f292c9f..2a2e6c58 100644 --- a/tests/multi_file/impl.cc +++ b/tests/multi_file/impl.cc @@ -12,106 +12,107 @@ OUTPUT: header.h "usr": "c:@S@Base", "short_name": "Base", "qualified_name": "Base", - "definition_spelling": "10:8-10:12", - "definition_extent": "10:1-10:15", + "definition_spelling": "3:8-3:12", + "definition_extent": "3:1-3:15", "derived": [1], - "uses": ["*10:8-10:12", "*12:26-12:30"] + "uses": ["*3:8-3:12", "*5:26-5:30"] }, { "id": 1, "usr": "c:@S@SameFileDerived", "short_name": "SameFileDerived", "qualified_name": "SameFileDerived", - "definition_spelling": "12:8-12:23", - "definition_extent": "12:1-12:33", + "definition_spelling": "5:8-5:23", + "definition_extent": "5:1-5:33", "parents": [0], - "uses": ["*12:8-12:23", "*14:14-14:29"] + "uses": ["*5:8-5:23", "*7:14-7:29"] }, { "id": 2, "usr": "c:@Foo0", "short_name": "Foo0", "qualified_name": "Foo0", - "definition_spelling": "14:7-14:11", - "definition_extent": "14:1-14:29", + "definition_spelling": "7:7-7:11", + "definition_extent": "7:1-7:29", "alias_of": 1, - "uses": ["*14:7-14:11"] + "uses": ["*7:7-7:11"] }, { "id": 3, "usr": "c:@ST>1#T@Foo2", "short_name": "Foo2", "qualified_name": "Foo2", - "definition_spelling": "20:8-20:12", - "definition_extent": "20:1-20:15", - "uses": ["*20:8-20:12"] + "definition_spelling": "13:8-13:12", + "definition_extent": "13:1-13:15", + "uses": ["*13:8-13:12"] }, { "id": 4, "usr": "c:@E@Foo3", "short_name": "Foo3", "qualified_name": "Foo3", - "definition_spelling": "22:6-22:10", - "definition_extent": "22:1-22:22", + "definition_spelling": "15:6-15:10", + "definition_extent": "15:1-15:22", "vars": [0, 1, 2], - "uses": ["*22:6-22:10"] + "uses": ["*15:6-15:10"] }], "funcs": [{ "id": 0, "usr": "c:@FT@>1#TFoo1#v#", "short_name": "Foo1", "qualified_name": "Foo1", - "definition_spelling": "17:6-17:10", - "definition_extent": "17:1-17:15", - "uses": ["17:6-17:10"] + "definition_spelling": "10:6-10:10", + "definition_extent": "10:1-10:15", + "uses": ["10:6-10:10"] }], "vars": [{ "id": 0, "usr": "c:@E@Foo3@A", "short_name": "A", "qualified_name": "Foo3::A", - "definition_spelling": "22:13-22:14", - "definition_extent": "22:13-22:14", + "definition_spelling": "15:13-15:14", + "definition_extent": "15:13-15:14", "variable_type": 4, "declaring_type": 4, - "uses": ["22:13-22:14"] + "uses": ["15:13-15:14"] }, { "id": 1, "usr": "c:@E@Foo3@B", "short_name": "B", "qualified_name": "Foo3::B", - "definition_spelling": "22:16-22:17", - "definition_extent": "22:16-22:17", + "definition_spelling": "15:16-15:17", + "definition_extent": "15:16-15:17", "variable_type": 4, "declaring_type": 4, - "uses": ["22:16-22:17"] + "uses": ["15:16-15:17"] }, { "id": 2, "usr": "c:@E@Foo3@C", "short_name": "C", "qualified_name": "Foo3::C", - "definition_spelling": "22:19-22:20", - "definition_extent": "22:19-22:20", + "definition_spelling": "15:19-15:20", + "definition_extent": "15:19-15:20", "variable_type": 4, "declaring_type": 4, - "uses": ["22:19-22:20"] + "uses": ["15:19-15:20"] }, { "id": 3, "usr": "c:@Foo4", "short_name": "Foo4", "qualified_name": "Foo4", - "definition_spelling": "24:5-24:9", - "definition_extent": "24:1-24:9", - "uses": ["24:5-24:9"] + "definition_spelling": "17:5-17:9", + "definition_extent": "17:1-17:9", + "uses": ["17:5-17:9"] }, { "id": 4, "usr": "c:header.h@Foo5", "short_name": "Foo5", "qualified_name": "Foo5", - "definition_spelling": "25:12-25:16", - "definition_extent": "25:1-25:16", - "uses": ["25:12-25:16"] + "definition_spelling": "18:12-18:16", + "definition_extent": "18:1-18:16", + "uses": ["18:12-18:16"] }] } OUTPUT: impl.cc { + "dependencies": ["C:/Users/jacob/Desktop/superindex/indexer/tests/multi_file/header.h"], "funcs": [{ "id": 0, "usr": "c:@F@Impl#", diff --git a/tests/multi_file/simple_impl.cc b/tests/multi_file/simple_impl.cc index 3fd48721..055f8e90 100644 --- a/tests/multi_file/simple_impl.cc +++ b/tests/multi_file/simple_impl.cc @@ -19,6 +19,7 @@ OUTPUT: simple_header.h OUTPUT: simple_impl.cc { + "dependencies": ["C:/Users/jacob/Desktop/superindex/indexer/tests/multi_file/simple_header.h"], "funcs": [{ "id": 0, "usr": "c:@F@impl#",