From afd38cbce95b3e0776089c308891391f9253695a Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Sun, 11 Feb 2018 20:22:47 -0800 Subject: [PATCH] Add Id file; to Use and simplify query.cc --- src/indexer.cc | 11 +++- src/indexer.h | 8 ++- src/messages/cquery_call_tree.cc | 12 ++-- src/messages/text_document_code_action.cc | 12 ++-- src/messages/text_document_code_lens.cc | 3 +- src/messages/text_document_definition.cc | 2 +- src/messages/text_document_highlight.cc | 2 +- src/messages/text_document_rename.cc | 16 +++--- src/query.cc | 67 +++++++++-------------- src/query.h | 5 -- src/query_utils.cc | 33 ++++------- src/query_utils.h | 2 +- 12 files changed, 76 insertions(+), 97 deletions(-) diff --git a/src/indexer.cc b/src/indexer.cc index c23ab8e2..020bb500 100644 --- a/src/indexer.cc +++ b/src/indexer.cc @@ -657,8 +657,8 @@ void OnIndexReference_Function(IndexFile* db, } // namespace // static -const int IndexFile::kMajorVersion = 12; -const int IndexFile::kMinorVersion = 1; +const int IndexFile::kMajorVersion = 13; +const int IndexFile::kMinorVersion = 0; IndexFile::IndexFile(const std::string& path, const std::string& contents) : id_cache(path), path(path), file_contents(contents) {} @@ -1542,7 +1542,9 @@ void OnIndexDeclaration(CXClientData client_data, const CXIdxDeclInfo* decl) { SetUse(db, &var->def.spell, decl_spell, lex_parent, Role::Definition); SetUse(db, &var->def.extent, decl_cursor.get_extent(), lex_parent, Role::None); } else { - var->declarations.push_back(decl_spell); + Maybe use; + SetUse(db, &use, decl_spell, lex_parent, Role::Declaration); + var->declarations.push_back(*use); } AddDeclInitializerUsages(db, decl_cursor); @@ -2374,6 +2376,9 @@ std::string GetClangVersion() { return ToString(clang_getClangVersion()); } +// |SymbolRef| is serialized this way. +// |Use| also uses this though it has an extra field |file|, +// which is not used by Index* so it does not need to be serialized. void Reflect(Reader& visitor, Reference& value) { if (visitor.Format() == SerializeFormat::Json) { std::string t = visitor.GetString(); diff --git a/src/indexer.h b/src/indexer.h index 7e6b189d..81aca123 100644 --- a/src/indexer.h +++ b/src/indexer.h @@ -136,13 +136,19 @@ struct SymbolRef : Reference { : Reference{Range(), si.id, si.kind, Role::None} {} }; +struct QueryFile; + // Represents an occurrence of a variable/type, |id,kind| refer to the lexical // parent. struct Use : Reference { + // |file| is used in Query* but not in Index* + Id file; Use() = default; Use(Reference ref) : Reference(ref) {} Use(Range range, Id id, SymbolKind kind, Role role) : Reference{range, id, kind, role} {} + Use(Range range, Id id, SymbolKind kind, Role role, Id file) + : Reference{range, id, kind, role}, file(file) {} }; void Reflect(Reader& visitor, Reference& value); @@ -433,7 +439,7 @@ struct IndexVar { Def def; - std::vector declarations; + std::vector declarations; // Usages. std::vector uses; diff --git a/src/messages/cquery_call_tree.cc b/src/messages/cquery_call_tree.cc index fe09d742..7bb7260d 100644 --- a/src/messages/cquery_call_tree.cc +++ b/src/messages/cquery_call_tree.cc @@ -80,7 +80,7 @@ std::vector BuildExpandCallTree( std::vector result; - auto handle_caller = [&](QueryFuncRef caller, + auto handle_caller = [&](Use caller, Out_CqueryCallTree::CallType call_type) { optional call_location = GetLsLocation(db, working_files, caller); @@ -130,21 +130,21 @@ std::vector BuildExpandCallTree( } }; - std::vector base_callers = + std::vector base_callers = GetCallersForAllBaseFunctions(db, root_func); - std::vector derived_callers = + std::vector derived_callers = GetCallersForAllDerivedFunctions(db, root_func); result.reserve(root_func.uses.size() + base_callers.size() + derived_callers.size()); - for (QueryFuncRef caller : root_func.uses) + for (Use caller : root_func.uses) handle_caller(caller, Out_CqueryCallTree::CallType::Direct); - for (QueryFuncRef caller : base_callers) + for (Use caller : base_callers) if (caller.kind == SymbolKind::Func && caller.id != Id(root)) { // Do not show calls to the base function coming from this function. handle_caller(caller, Out_CqueryCallTree::CallType::Base); } - for (QueryFuncRef caller : derived_callers) + for (Use caller : derived_callers) handle_caller(caller, Out_CqueryCallTree::CallType::Derived); return result; diff --git a/src/messages/text_document_code_action.cc b/src/messages/text_document_code_action.cc index 1dd62ca4..f7f0d6dc 100644 --- a/src/messages/text_document_code_action.cc +++ b/src/messages/text_document_code_action.cc @@ -73,7 +73,7 @@ optional GetImplementationFile(QueryDatabase* db, // Note: we ignore the definition if it is in the same file (ie, // possibly a header). if (func.def && func.def->extent) { - Maybe t = db->GetFileId(*func.def->extent); + QueryFileId t = func.def->extent->file; if (t != file_id) return t; } @@ -84,7 +84,7 @@ optional GetImplementationFile(QueryDatabase* db, // Note: we ignore the definition if it is in the same file (ie, // possibly a header). if (var.def && var.def->extent) { - Maybe t = db->GetFileId(*var.def->extent); + QueryFileId t = var.def->extent->file; if (t != file_id) return t; } @@ -150,8 +150,8 @@ optional BuildAutoImplementForFunction(QueryDatabase* db, QueryFileId impl_file_id, QueryFunc& func) { assert(func.def); - for (const Reference& decl : func.declarations) { - if (db->GetFileId(decl) != decl_file_id) + for (Use decl : func.declarations) { + if (decl.file != decl_file_id) continue; optional ls_decl = GetLsRange(working_file, decl.range); @@ -204,8 +204,8 @@ optional BuildAutoImplementForFunction(QueryDatabase* db, if (!sym_func.def || !sym_func.def->extent) break; - for (const Reference& func_decl : sym_func.declarations) { - if (db->GetFileId(func_decl) == decl_file_id) { + for (Use func_decl : sym_func.declarations) { + if (func_decl.file == decl_file_id) { int dist = func_decl.range.start.line - decl.range.start.line; if (abs(dist) < abs(best_dist)) { optional def_loc = GetLsLocation( diff --git a/src/messages/text_document_code_lens.cc b/src/messages/text_document_code_lens.cc index 529bb569..3eea01d9 100644 --- a/src/messages/text_document_code_lens.cc +++ b/src/messages/text_document_code_lens.cc @@ -91,6 +91,7 @@ void AddCodeLens(const char* singular, optional range = GetLsRange(common->working_file, loc.range); if (!range) return; + // FIXME SymbolRef loc -> Use loc Maybe file_id = common->db->GetFileId(loc); if (!file_id) return; @@ -178,7 +179,7 @@ struct TextDocumentCodeLensHandler // extent location to the spelling location. auto try_ensure_spelling = [&](SymbolRef sym) { Maybe def = GetDefinitionSpellingOfSymbol(db, sym); - if (!def || db->GetFileId(*def) != db->GetFileId(sym) || + if (!def || def->file != *db->GetFileId(sym) || def->range.start.line != sym.range.start.line) { return sym; } diff --git a/src/messages/text_document_definition.cc b/src/messages/text_document_definition.cc index 048723b9..3ef4b6bf 100644 --- a/src/messages/text_document_definition.cc +++ b/src/messages/text_document_definition.cc @@ -86,7 +86,7 @@ struct TextDocumentDefinitionHandler // the declaration if possible. We also want to use declarations if // we're pointing to, ie, a pure virtual function which has no // definition. - if (!def_loc || (db->GetFileId(*def_loc) == file_id && + if (!def_loc || (def_loc->file == file_id && def_loc->range.Contains(target_line, target_column))) { // Goto declaration. diff --git a/src/messages/text_document_highlight.cc b/src/messages/text_document_highlight.cc index 88d8db6d..5babbc20 100644 --- a/src/messages/text_document_highlight.cc +++ b/src/messages/text_document_highlight.cc @@ -41,7 +41,7 @@ struct TextDocumentDocumentHighlightHandler std::vector uses = GetUsesOfSymbol(db, sym, true); out.result.reserve(uses.size()); for (Use use : uses) { - if (db->GetFileId(use) != file_id) + if (use.file != file_id) continue; optional ls_location = diff --git a/src/messages/text_document_rename.cc b/src/messages/text_document_rename.cc index 711985a3..92a83755 100644 --- a/src/messages/text_document_rename.cc +++ b/src/messages/text_document_rename.cc @@ -16,23 +16,21 @@ lsWorkspaceEdit BuildWorkspaceEdit(QueryDatabase* db, if (!ls_location) continue; - Maybe file_id = db->GetFileId(use); - if (!file_id) - continue; - if (path_to_edit.find(*file_id) == path_to_edit.end()) { - path_to_edit[*file_id] = lsTextDocumentEdit(); + QueryFileId file_id = use.file; + if (path_to_edit.find(file_id) == path_to_edit.end()) { + path_to_edit[file_id] = lsTextDocumentEdit(); - QueryFile& file = db->files[file_id->id]; + QueryFile& file = db->files[file_id.id]; if (!file.def) continue; const std::string& path = file.def->path; - path_to_edit[*file_id].textDocument.uri = + path_to_edit[file_id].textDocument.uri = lsDocumentUri::FromPath(path); WorkingFile* working_file = working_files->GetFileByFilename(path); if (working_file) - path_to_edit[*file_id].textDocument.version = + path_to_edit[file_id].textDocument.version = working_file->version; } @@ -41,7 +39,7 @@ lsWorkspaceEdit BuildWorkspaceEdit(QueryDatabase* db, edit.newText = new_text; // vscode complains if we submit overlapping text edits. - auto& edits = path_to_edit[*file_id].edits; + auto& edits = path_to_edit[file_id].edits; if (std::find(edits.begin(), edits.end(), edit) == edits.end()) edits.push_back(edit); } diff --git a/src/query.cc b/src/query.cc index 873036d5..04fe1d61 100644 --- a/src/query.cc +++ b/src/query.cc @@ -233,10 +233,6 @@ QueryFile::DefUpdate BuildFileDefUpdate(const IdMap& id_map, const IndexFile& in Role role) { def.outline.push_back(SymbolRef(range, id, kind, role)); }; - auto add_all_symbols = [&](Range range, Id id, SymbolKind kind, - Role role) { - def.all_symbols.push_back(SymbolRef(range, id, kind, role)); - }; auto add_all_symbols_use = [&](Use use, Id id, SymbolKind kind) { def.all_symbols.push_back( SymbolRef(use.range, id, kind, use.role)); @@ -252,7 +248,7 @@ QueryFile::DefUpdate BuildFileDefUpdate(const IdMap& id_map, const IndexFile& in if (type.def.extent) add_outline_use(*type.def.extent, id, SymbolKind::Type); for (Use use : type.uses) - add_all_symbols(use.range, id, SymbolKind::Type, use.role); + add_all_symbols_use(use, id, SymbolKind::Type); } for (const IndexFunc& func : indexed.funcs) { QueryFuncId id = id_map.ToQuery(func.id); @@ -261,23 +257,21 @@ QueryFile::DefUpdate BuildFileDefUpdate(const IdMap& id_map, const IndexFile& in if (func.def.extent) add_outline_use(*func.def.extent, id, SymbolKind::Func); for (const IndexFunc::Declaration& decl : func.declarations) { - add_all_symbols(decl.spelling, id, SymbolKind::Func, - Role::Declaration); + def.all_symbols.push_back( + SymbolRef(decl.spelling, id, SymbolKind::Func, Role::Declaration)); add_outline(decl.spelling, id, SymbolKind::Func, Role::Declaration); } - for (Use caller : func.uses) { + for (Use use : func.uses) { // Make ranges of implicit function calls larger (spanning one more column // to the left/right). This is hacky but useful. e.g. // textDocument/definition on the space/semicolon in `A a;` or `return // 42;` will take you to the constructor. - Range range = caller.range; - if (caller.role & Role::Implicit) { - if (range.start.column > 0) - range.start.column--; - range.end.column++; + if (use.role & Role::Implicit) { + if (use.range.start.column > 0) + use.range.start.column--; + use.range.end.column++; } - add_all_symbols(range, id, SymbolKind::Func, - caller.role | Role::Call); + add_all_symbols_use(use, id, SymbolKind::Func); } } for (const IndexVar& var : indexed.vars) { @@ -286,15 +280,12 @@ QueryFile::DefUpdate BuildFileDefUpdate(const IdMap& id_map, const IndexFile& in add_all_symbols_use(*var.def.spell, id, SymbolKind::Var); if (var.def.extent) add_outline_use(*var.def.extent, id, SymbolKind::Var); - for (const Range& decl : var.declarations) { - add_all_symbols(decl, id_map.ToQuery(var.id), SymbolKind::Var, - Role::Definition); - add_outline(decl, id_map.ToQuery(var.id), SymbolKind::Var, - Role::Declaration); + for (Use decl : var.declarations) { + add_all_symbols_use(decl, id, SymbolKind::Var); + add_outline_use(decl, id, SymbolKind::Var); } for (Use use : var.uses) - add_all_symbols(use.range, id_map.ToQuery(var.id), SymbolKind::Var, - use.role); + add_all_symbols_use(use, id, SymbolKind::Var); } std::sort(def.outline.begin(), def.outline.end(), @@ -411,14 +402,12 @@ IdMap::IdMap(QueryDatabase* query_db, const IdCache& local_ids) *GetQueryVarIdFromUsr(query_db, entry.second, true); } -Use IdMap::ToQuery(Range range, Role role) const { - return Use(range, primary_file, SymbolKind:: File, role); -} QueryTypeId IdMap::ToQuery(IndexTypeId id) const { assert(cached_type_ids_.find(id) != cached_type_ids_.end()); return QueryTypeId(cached_type_ids_.find(id)->second); } QueryFuncId IdMap::ToQuery(IndexFuncId id) const { + // FIXME id shouldn't be invalid if (id == IndexFuncId()) return QueryFuncId(); assert(cached_func_ids_.find(id) != cached_func_ids_.end()); @@ -430,44 +419,38 @@ QueryVarId IdMap::ToQuery(IndexVarId id) const { } Use IdMap::ToQuery(Reference ref) const { + Use ret(ref); + ret.file = primary_file; switch (ref.kind) { case SymbolKind::Invalid: break; case SymbolKind::File: - ref.id = primary_file; + ret.id = primary_file; break; case SymbolKind::Func: - ref.id = ToQuery(IndexFuncId(ref.id)); + ret.id = ToQuery(IndexFuncId(ref.id)); break; case SymbolKind::Type: - ref.id = ToQuery(IndexTypeId(ref.id)); + ret.id = ToQuery(IndexTypeId(ref.id)); break; case SymbolKind::Var: - ref.id = ToQuery(IndexVarId(ref.id)); + ret.id = ToQuery(IndexVarId(ref.id)); break; } - return ref; + return ret; } SymbolRef IdMap::ToQuery(SymbolRef ref) const { ref.Reference::operator=(ToQuery(static_cast(ref))); return ref; } Use IdMap::ToQuery(Use use) const { - use.Reference::operator=(ToQuery(static_cast(use))); - return use; + return ToQuery(static_cast(use)); } Use IdMap::ToQuery(IndexFunc::Declaration decl) const { // TODO: expose more than just QueryLocation. - return Use(decl.spelling, primary_file, SymbolKind::File, - Role::Declaration); -} -std::vector IdMap::ToQuery(const std::vector& a) const { - std::vector ret; - ret.reserve(a.size()); - for (auto& x : a) - ret.push_back(ToQuery(x, Role::Reference)); - return ret; + return {decl.spelling, primary_file, SymbolKind::File, Role::Declaration, + primary_file}; } // ---------------------- @@ -1241,4 +1224,4 @@ TEST_SUITE("query") { // FIXME/TODO: See https://github.com/cquery-project/cquery/issues/443. // REQUIRE(db.vars[0].uses.size() == 0); } -} \ No newline at end of file +} diff --git a/src/query.h b/src/query.h index a43c0b00..f1fa156b 100644 --- a/src/query.h +++ b/src/query.h @@ -20,8 +20,6 @@ using QueryVarId = Id; struct IdMap; -using QueryFuncRef = Use; - // There are two sources of reindex updates: the (single) definition of a // symbol has changed, or one of many users of the symbol has changed. // @@ -341,7 +339,6 @@ template <> struct IndexToQuery { using type = QueryTypeId; }; template <> struct IndexToQuery { using type = QueryVarId; }; template <> struct IndexToQuery { using type = Use; }; template <> struct IndexToQuery { using type = SymbolRef; }; -template <> struct IndexToQuery { using type = Use; }; template <> struct IndexToQuery { using type = Use; }; template struct IndexToQuery> { using type = optional::type>; @@ -363,7 +360,6 @@ struct IdMap { QueryFuncId ToQuery(IndexFuncId id) const; QueryVarId ToQuery(IndexVarId id) const; SymbolRef ToQuery(SymbolRef ref) const; - Use ToQuery(Range range, Role role) const; Use ToQuery(Reference ref) const; Use ToQuery(Use ref) const; Use ToQuery(IndexFunc::Declaration decl) const; @@ -381,7 +377,6 @@ struct IdMap { ret.push_back(ToQuery(x)); return ret; } - std::vector ToQuery(const std::vector& a) const; // clang-format on diff --git a/src/query_utils.cc b/src/query_utils.cc index 483f0f3f..2888bd45 100644 --- a/src/query_utils.cc +++ b/src/query_utils.cc @@ -16,19 +16,13 @@ int ComputeRangeSize(const Range& range) { return range.end.column - range.start.column; } -Maybe UseWithFileId(Maybe use, QueryFileId file_id) { - if (!use) - return nullopt; - return Use(use->range, file_id, SymbolKind::File, use->role); -} - } // namespace Maybe GetDefinitionSpellingOfSymbol(QueryDatabase* db, QueryFuncId id) { QueryFunc& func = db->funcs[id.id]; if (func.def) - return UseWithFileId(func.def->spell, func.def->file); + return func.def->spell; return nullopt; } @@ -38,19 +32,19 @@ Maybe GetDefinitionSpellingOfSymbol(QueryDatabase* db, case SymbolKind::Type: { QueryType& type = db->GetType(sym); if (type.def) - return UseWithFileId(type.def->spell, type.def->file); + return type.def->spell; break; } case SymbolKind::Func: { QueryFunc& func = db->GetFunc(sym); if (func.def) - return UseWithFileId(func.def->spell, func.def->file); + return func.def->spell; break; } case SymbolKind::Var: { QueryVar& var = db->GetVar(sym); if (var.def) - return UseWithFileId(var.def->spell, var.def->file); + return var.def->spell; break; } case SymbolKind::File: @@ -67,24 +61,24 @@ Maybe GetDefinitionExtentOfSymbol(QueryDatabase* db, SymbolIdx sym) { case SymbolKind::Type: { QueryType& type = db->GetType(sym); if (type.def) - return UseWithFileId(type.def->extent, type.def->file); + return type.def->extent; break; } case SymbolKind::Func: { QueryFunc& func = db->GetFunc(sym); if (func.def) - return UseWithFileId(func.def->extent, func.def->file); + return func.def->extent; break; } case SymbolKind::Var: { QueryVar& var = db->GetVar(sym); if (var.def) - return UseWithFileId(var.def->extent, var.def->file); + return var.def->extent; break; } case SymbolKind::File: return Use(Range(Position(0, 0), Position(0, 0)), sym.id, sym.kind, - Role::None); + Role::None, QueryFileId(sym.id)); case SymbolKind::Invalid: { assert(false && "unexpected"); break; @@ -105,7 +99,7 @@ Maybe GetDeclarationFileForSymbol(QueryDatabase* db, case SymbolKind::Func: { QueryFunc& func = db->GetFunc(sym); if (!func.declarations.empty()) - return db->GetFileId(func.declarations[0]); + return func.declarations[0].file; if (func.def) return func.def->file; break; @@ -383,14 +377,11 @@ lsDocumentUri GetLsDocumentUri(QueryDatabase* db, QueryFileId file_id) { optional GetLsLocation(QueryDatabase* db, WorkingFiles* working_files, - Reference ref) { + Use use) { std::string path; - Maybe file_id = db->GetFileId(ref); - if (!file_id) - return nullopt; - lsDocumentUri uri = GetLsDocumentUri(db, *file_id, &path); + lsDocumentUri uri = GetLsDocumentUri(db, use.file, &path); optional range = - GetLsRange(working_files->GetFileByFilename(path), ref.range); + GetLsRange(working_files->GetFileByFilename(path), use.range); if (!range) return nullopt; return lsLocation(uri, *range); diff --git a/src/query_utils.h b/src/query_utils.h index 8d5f43be..e4b38d25 100644 --- a/src/query_utils.h +++ b/src/query_utils.h @@ -44,7 +44,7 @@ lsDocumentUri GetLsDocumentUri(QueryDatabase* db, QueryFileId file_id); optional GetLsLocation(QueryDatabase* db, WorkingFiles* working_files, - Reference location); + Use use); optional GetLsLocationEx(QueryDatabase* db, WorkingFiles* working_files, Use use,