From 5fbe4eac830dc7f5f2daa6b4e56c38ba9473e028 Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Sun, 14 Oct 2018 23:33:24 -0700 Subject: [PATCH] Add ExtentRef; merge symbol2refcnt and outline2refcnt Fix hierarchical document symbol for namespaces when there are multiple declarations. --- src/indexer.h | 9 ++ src/messages/ccls_call.cc | 19 +-- src/messages/ccls_navigate.cc | 54 ++++----- src/messages/textDocument_codeLens.cc | 41 +++---- src/messages/textDocument_documentSymbol.cc | 121 +++++++++----------- src/query.cc | 28 ++--- src/query.h | 20 ++-- 7 files changed, 136 insertions(+), 156 deletions(-) diff --git a/src/indexer.h b/src/indexer.h index 2d263866..2124317a 100644 --- a/src/indexer.h +++ b/src/indexer.h @@ -51,6 +51,15 @@ struct SymbolRef { }; MAKE_HASHABLE(SymbolRef, t.range, t.usr, t.kind, t.role); +struct ExtentRef : SymbolRef { + Range extent; + std::tuple ToTuple() const { + return std::make_tuple(range, usr, kind, role, extent); + } + bool operator==(const ExtentRef &o) const { return ToTuple() == o.ToTuple(); } +}; +MAKE_HASHABLE(ExtentRef, t.range, t.usr, t.kind, t.role, t.extent); + struct Ref { Range range; Role role; diff --git a/src/messages/ccls_call.cc b/src/messages/ccls_call.cc index 54de411f..dc6f57c1 100644 --- a/src/messages/ccls_call.cc +++ b/src/messages/ccls_call.cc @@ -102,15 +102,16 @@ bool Expand(MessageHandler *m, Out_cclsCall *entry, bool callee, } else { for (Use use : func.uses) { const QueryFile &file1 = m->db->files[use.file_id]; - Maybe best_sym; - for (auto [sym, refcnt] : file1.outline2refcnt) - if (refcnt > 0 && sym.kind == SymbolKind::Func && - sym.range.start <= use.range.start && - use.range.end <= sym.range.end && - (!best_sym || best_sym->range.start < sym.range.start)) - best_sym = sym; - if (best_sym) - handle(*best_sym, use.file_id, call_type); + Maybe best; + for (auto [sym, refcnt] : file1.symbol2refcnt) + if (refcnt > 0 && sym.extent.Valid() && + sym.kind == SymbolKind::Func && + sym.extent.start <= use.range.start && + use.range.end <= sym.extent.end && + (!best || best->extent.start < sym.extent.start)) + best = sym; + if (best) + handle(*best, use.file_id, call_type); } } }; diff --git a/src/messages/ccls_navigate.cc b/src/messages/ccls_navigate.cc index 9ab73178..e2528be8 100644 --- a/src/messages/ccls_navigate.cc +++ b/src/messages/ccls_navigate.cc @@ -43,49 +43,49 @@ struct Handler_CclsNavigate : BaseMessageHandler { switch (params.direction[0]) { case 'D': { Maybe parent; - for (auto [sym, refcnt] : file->outline2refcnt) - if (refcnt > 0 && sym.range.start <= pos && pos < sym.range.end && - (!parent || parent->start < sym.range.start)) - parent = sym.range; - for (auto [sym, refcnt] : file->outline2refcnt) - if (refcnt > 0 && pos < sym.range.start && - (!parent || sym.range.end <= parent->end) && - (!res || sym.range.start < res->start)) - res = sym.range; + for (auto [sym, refcnt] : file->symbol2refcnt) + if (refcnt > 0 && sym.extent.Valid() && sym.extent.start <= pos && + pos < sym.extent.end && (!parent || parent->start < sym.extent.start)) + parent = sym.extent; + for (auto [sym, refcnt] : file->symbol2refcnt) + if (refcnt > 0 && pos < sym.extent.start && + (!parent || sym.extent.end <= parent->end) && + (!res || sym.extent.start < res->start)) + res = sym.extent; break; } case 'L': - for (auto [sym, refcnt] : file->outline2refcnt) - if (refcnt > 0 && sym.range.end <= pos && - (!res || (res->end == sym.range.end ? sym.range.start < res->start - : res->end < sym.range.end))) - res = sym.range; + for (auto [sym, refcnt] : file->symbol2refcnt) + if (refcnt > 0 && sym.extent.Valid() && sym.extent.end <= pos && + (!res || (res->end == sym.extent.end ? sym.extent.start < res->start + : res->end < sym.extent.end))) + res = sym.extent; break; case 'R': { Maybe parent; - for (auto [sym, refcnt] : file->outline2refcnt) - if (refcnt > 0 && sym.range.start <= pos && pos < sym.range.end && - (!parent || parent->start < sym.range.start)) - parent = sym.range; + for (auto [sym, refcnt] : file->symbol2refcnt) + if (refcnt > 0 && sym.extent.Valid() && sym.extent.start <= pos && + pos < sym.extent.end && (!parent || parent->start < sym.extent.start)) + parent = sym.extent; if (parent && parent->start.line == pos.line && pos < parent->end) { pos = parent->end; if (pos.column) pos.column--; } - for (auto [sym, refcnt] : file->outline2refcnt) - if (refcnt > 0 && pos < sym.range.start && + for (auto [sym, refcnt] : file->symbol2refcnt) + if (refcnt > 0 && sym.extent.Valid() && pos < sym.extent.start && (!res || - (sym.range.start == res->start ? res->end < sym.range.end - : sym.range.start < res->start))) - res = sym.range; + (sym.extent.start == res->start ? res->end < sym.extent.end + : sym.extent.start < res->start))) + res = sym.extent; break; } case 'U': default: - for (auto [sym, refcnt] : file->outline2refcnt) - if (refcnt > 0 && sym.range.start < pos && pos < sym.range.end && - (!res || res->start < sym.range.start)) - res = sym.range; + for (auto [sym, refcnt] : file->symbol2refcnt) + if (refcnt > 0 && sym.extent.Valid() && sym.extent.start < pos && + pos < sym.extent.end && (!res || res->start < sym.extent.start)) + res = sym.extent; break; } std::vector result; diff --git a/src/messages/textDocument_codeLens.cc b/src/messages/textDocument_codeLens.cc index a0c1fc2e..23648250 100644 --- a/src/messages/textDocument_codeLens.cc +++ b/src/messages/textDocument_codeLens.cc @@ -74,15 +74,15 @@ struct Handler_TextDocumentCodeLens return; WorkingFile *wfile = working_files->GetFileByFilename(file->def->path); - auto Add = [&](const char *singular, Cmd_xref show, Use use, int num, + auto Add = [&](const char *singular, Cmd_xref show, Range range, int num, bool force_display = false) { if (!num && !force_display) return; - std::optional range = GetLsRange(wfile, use.range); - if (!range) + std::optional ls_range = GetLsRange(wfile, range); + if (!ls_range) return; lsCodeLens &code_lens = result.emplace_back(); - code_lens.range = *range; + code_lens.range = *ls_range; code_lens.command = lsCommand(); code_lens.command->command = std::string(ccls_xref); bool plural = num > 1 && singular[strlen(singular) - 1] != 'd'; @@ -91,19 +91,10 @@ struct Handler_TextDocumentCodeLens code_lens.command->arguments.push_back(ToString(show)); }; - auto ToSpell = [&](SymbolRef sym, int file_id) -> Use { - Maybe def = GetDefinitionSpell(db, sym); - if (def && def->file_id == file_id && - def->range.start.line == sym.range.start.line) - return *def; - return {{sym.range, sym.role}, file_id}; - }; - std::unordered_set seen; - for (auto [sym, refcnt] : file->outline2refcnt) { - if (refcnt <= 0 || !seen.insert(sym.range).second) + for (auto [sym, refcnt] : file->symbol2refcnt) { + if (refcnt <= 0 || !sym.extent.Valid() || !seen.insert(sym.range).second) continue; - Use use = ToSpell(sym, file->id); switch (sym.kind) { case SymbolKind::Func: { QueryFunc &func = db->GetFunc(sym); @@ -112,28 +103,28 @@ struct Handler_TextDocumentCodeLens continue; std::vector base_uses = GetUsesForAllBases(db, func); std::vector derived_uses = GetUsesForAllDerived(db, func); - Add("ref", {sym.usr, SymbolKind::Func, "uses"}, use, func.uses.size(), - base_uses.empty()); + Add("ref", {sym.usr, SymbolKind::Func, "uses"}, sym.range, + func.uses.size(), base_uses.empty()); if (base_uses.size()) - Add("b.ref", {sym.usr, SymbolKind::Func, "bases uses"}, use, + Add("b.ref", {sym.usr, SymbolKind::Func, "bases uses"}, sym.range, base_uses.size()); if (derived_uses.size()) - Add("d.ref", {sym.usr, SymbolKind::Func, "derived uses"}, use, + Add("d.ref", {sym.usr, SymbolKind::Func, "derived uses"}, sym.range, derived_uses.size()); if (base_uses.empty()) - Add("base", {sym.usr, SymbolKind::Func, "bases"}, use, + Add("base", {sym.usr, SymbolKind::Func, "bases"}, sym.range, def->bases.size()); - Add("derived", {sym.usr, SymbolKind::Func, "derived"}, use, + Add("derived", {sym.usr, SymbolKind::Func, "derived"}, sym.range, func.derived.size()); break; } case SymbolKind::Type: { QueryType &type = db->GetType(sym); - Add("ref", {sym.usr, SymbolKind::Type, "uses"}, use, type.uses.size(), + Add("ref", {sym.usr, SymbolKind::Type, "uses"}, sym.range, type.uses.size(), true); - Add("derived", {sym.usr, SymbolKind::Type, "derived"}, use, + Add("derived", {sym.usr, SymbolKind::Type, "derived"}, sym.range, type.derived.size()); - Add("var", {sym.usr, SymbolKind::Type, "instances"}, use, + Add("var", {sym.usr, SymbolKind::Type, "instances"}, sym.range, type.instances.size()); break; } @@ -142,7 +133,7 @@ struct Handler_TextDocumentCodeLens const QueryVar::Def *def = var.AnyDef(); if (!def || (def->is_local() && !g_config->codeLens.localVariables)) continue; - Add("ref", {sym.usr, SymbolKind::Var, "uses"}, use, var.uses.size(), + Add("ref", {sym.usr, SymbolKind::Var, "uses"}, sym.range, var.uses.size(), def->kind != lsSymbolKind::Macro); break; } diff --git a/src/messages/textDocument_documentSymbol.cc b/src/messages/textDocument_documentSymbol.cc index e58aba80..6047ae0e 100644 --- a/src/messages/textDocument_documentSymbol.cc +++ b/src/messages/textDocument_documentSymbol.cc @@ -71,12 +71,11 @@ struct Handler_TextDocumentDocumentSymbol if (!wfile) return; - const auto &symbol2refcnt = - params.all ? file->symbol2refcnt : file->outline2refcnt; if (params.startLine >= 0) { std::vector result; - for (auto [sym, refcnt] : symbol2refcnt) - if (refcnt > 0 && params.startLine <= sym.range.start.line && + for (auto [sym, refcnt] : file->symbol2refcnt) + if (refcnt > 0 && (params.all || sym.extent.Valid()) && + params.startLine <= sym.range.start.line && sym.range.start.line <= params.endLine) if (auto loc = GetLsLocation(db, working_files, sym, file_id)) result.push_back(loc->range); @@ -84,95 +83,75 @@ struct Handler_TextDocumentDocumentSymbol pipeline::Reply(request->id, result); } else if (g_config->client.hierarchicalDocumentSymbolSupport) { std::unordered_map> sym2ds; - std::vector> funcs; - std::vector> types; - for (auto [sym, refcnt] : symbol2refcnt) { - if (refcnt <= 0) + std::vector, lsDocumentSymbol *>> + funcs, types; + for (auto [sym, refcnt] : file->symbol2refcnt) { + if (refcnt <= 0 || !sym.extent.Valid()) continue; auto r = sym2ds.try_emplace(SymbolIdx{sym.usr, sym.kind}); if (!r.second) continue; auto &ds = r.first->second; ds = std::make_unique(); - const void *def_ptr = nullptr; - WithEntity(db, sym, [&](const auto &entity) { + std::vector def_ptrs; + WithEntity(db, sym, [&, sym = sym](const auto &entity) { auto *def = entity.AnyDef(); if (!def) return; ds->name = def->Name(false); ds->detail = def->Name(true); + if (auto ls_range = GetLsRange(wfile, sym.range)) { + ds->selectionRange = *ls_range; + ds->range = ds->selectionRange; + if (sym.extent.Valid()) + if (auto ls_range1 = GetLsRange(wfile, sym.extent)) + ds->range = *ls_range1; + } - // Try to find a definition with spell first. - const void *candidate_def_ptr = nullptr; for (auto &def : entity.def) if (def.file_id == file_id && !Ignore(&def)) { ds->kind = def.kind; - if (def.kind == lsSymbolKind::Namespace) - candidate_def_ptr = &def; - - if (!def.spell) - continue; - if (auto ls_range = GetLsRange(wfile, def.spell->extent)) - ds->range = *ls_range; - else - continue; - if (auto ls_range = GetLsRange(wfile, def.spell->range)) - ds->selectionRange = *ls_range; - else - continue; - def_ptr = &def; - break; + if (def.spell || def.kind == lsSymbolKind::Namespace) + def_ptrs.push_back(&def); } - - // Try to find a declaration. - if (!def_ptr && candidate_def_ptr) - for (auto &decl : entity.declarations) - if (decl.file_id == file_id) { - if (auto ls_range = GetLsRange(wfile, decl.extent)) - ds->range = *ls_range; - else - continue; - if (auto ls_range = GetLsRange(wfile, decl.range)) - ds->selectionRange = *ls_range; - else - continue; - def_ptr = candidate_def_ptr; - break; - } }); - if (!def_ptr) { + if (def_ptrs.empty() || !(params.all || sym.role & Role::Definition || + ds->kind == lsSymbolKind::Namespace)) { ds.reset(); continue; } if (sym.kind == SymbolKind::Func) - funcs.emplace_back((const QueryFunc::Def *)def_ptr, ds.get()); + funcs.emplace_back(std::move(def_ptrs), ds.get()); else if (sym.kind == SymbolKind::Type) - types.emplace_back((const QueryType::Def *)def_ptr, ds.get()); + types.emplace_back(std::move(def_ptrs), ds.get()); } - for (auto &[def, ds] : funcs) - for (Usr usr1 : def->vars) { - auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Var}); - if (it != sym2ds.end() && it->second) - ds->children.push_back(std::move(it->second)); + for (auto &[def_ptrs, ds] : funcs) + for (const void *def_ptr : def_ptrs) + for (Usr usr1 : ((const QueryFunc::Def *)def_ptr)->vars) { + auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Var}); + if (it != sym2ds.end() && it->second) + ds->children.push_back(std::move(it->second)); + } + for (auto &[def_ptrs, ds] : types) + for (const void *def_ptr : def_ptrs) { + auto *def = (const QueryType::Def *)def_ptr; + for (Usr usr1 : def->funcs) { + auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Func}); + if (it != sym2ds.end() && it->second) + ds->children.push_back(std::move(it->second)); + } + for (Usr usr1 : def->types) { + auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Type}); + if (it != sym2ds.end() && it->second) + ds->children.push_back(std::move(it->second)); + } + for (auto [usr1, _] : def->vars) { + auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Var}); + if (it != sym2ds.end() && it->second) + ds->children.push_back(std::move(it->second)); + } } - for (auto &[def, ds] : types) { - for (Usr usr1 : def->funcs) { - auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Func}); - if (it != sym2ds.end() && it->second) - ds->children.push_back(std::move(it->second)); - } - for (Usr usr1 : def->types) { - auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Type}); - if (it != sym2ds.end() && it->second) - ds->children.push_back(std::move(it->second)); - } - for (auto [usr1, _] : def->vars) { - auto it = sym2ds.find(SymbolIdx{usr1, SymbolKind::Var}); - if (it != sym2ds.end() && it->second) - ds->children.push_back(std::move(it->second)); - } - } std::vector> result; for (auto &[_, ds] : sym2ds) if (ds) @@ -180,8 +159,10 @@ struct Handler_TextDocumentDocumentSymbol pipeline::Reply(request->id, result); } else { std::vector result; - for (auto [sym, refcnt] : symbol2refcnt) { - if (refcnt <= 0) continue; + for (auto [sym, refcnt] : file->symbol2refcnt) { + if (refcnt <= 0 || !sym.extent.Valid() || + !(params.all || sym.role & Role::Definition)) + continue; if (std::optional info = GetSymbolInfo(db, sym, false)) { if ((sym.kind == SymbolKind::Type && diff --git a/src/query.cc b/src/query.cc index d916f073..6d2ab2e8 100644 --- a/src/query.cc +++ b/src/query.cc @@ -224,7 +224,7 @@ void DB::ApplyIndexUpdate(IndexUpdate *u) { SymbolKind kind, Use &use, int delta) { use.file_id = use.file_id == -1 ? u->file_id : lid2fid.find(use.file_id)->second; - SymbolRef sym{use.range, usr, kind, use.role}; + ExtentRef sym{{use.range, usr, kind, use.role}}; int &v = files[use.file_id].symbol2refcnt[sym]; v += delta; assert(v >= 0); @@ -233,9 +233,14 @@ void DB::ApplyIndexUpdate(IndexUpdate *u) { }; auto RefDecl = [&](std::unordered_map &lid2fid, Usr usr, SymbolKind kind, DeclRef &dr, int delta) { - Ref(lid2fid, usr, kind, dr, delta); - files[dr.file_id] - .outline2refcnt[SymbolRef{dr.extent, usr, kind, dr.role}] += delta; + dr.file_id = + dr.file_id == -1 ? u->file_id : lid2fid.find(dr.file_id)->second; + ExtentRef sym{{dr.range, usr, kind, dr.role}, dr.extent}; + int &v = files[dr.file_id].symbol2refcnt[sym]; + v += delta; + assert(v >= 0); + if (!v) + files[dr.file_id].symbol2refcnt.erase(sym); }; auto UpdateUses = @@ -369,9 +374,8 @@ void DB::Update(const Lid2file_id &lid2file_id, int file_id, if (def.spell) { AssignFileId(lid2file_id, file_id, *def.spell); files[def.spell->file_id].symbol2refcnt[{ - def.spell->range, u.first, SymbolKind::Func, def.spell->role}]++; - files[def.spell->file_id].outline2refcnt[{ - def.spell->extent, u.first, SymbolKind::Func, def.spell->role}]++; + {def.spell->range, u.first, SymbolKind::Func, def.spell->role}, + def.spell->extent}]++; } auto R = func_usr.try_emplace({u.first}, func_usr.size()); @@ -393,9 +397,8 @@ void DB::Update(const Lid2file_id &lid2file_id, int file_id, if (def.spell) { AssignFileId(lid2file_id, file_id, *def.spell); files[def.spell->file_id].symbol2refcnt[{ - def.spell->range, u.first, SymbolKind::Type, def.spell->role}]++; - files[def.spell->file_id].outline2refcnt[{ - def.spell->extent, u.first, SymbolKind::Type, def.spell->role}]++; + {def.spell->range, u.first, SymbolKind::Type, def.spell->role}, + def.spell->extent}]++; } auto R = type_usr.try_emplace({u.first}, type_usr.size()); if (R.second) @@ -416,9 +419,8 @@ void DB::Update(const Lid2file_id &lid2file_id, int file_id, if (def.spell) { AssignFileId(lid2file_id, file_id, *def.spell); files[def.spell->file_id].symbol2refcnt[{ - def.spell->range, u.first, SymbolKind::Var, def.spell->role}]++; - files[def.spell->file_id].outline2refcnt[{ - def.spell->extent, u.first, SymbolKind::Var, def.spell->role}]++; + {def.spell->range, u.first, SymbolKind::Var, def.spell->role}, + def.spell->extent}]++; } auto R = var_usr.try_emplace({u.first}, var_usr.size()); if (R.second) diff --git a/src/query.h b/src/query.h index 78cf90ee..a64d2420 100644 --- a/src/query.h +++ b/src/query.h @@ -11,17 +11,13 @@ #include namespace llvm { -template <> struct DenseMapInfo { - static inline SymbolRef getEmptyKey() { return {}; } - static inline SymbolRef getTombstoneKey() { - SymbolRef ret{}; - ret.usr = -1; - return ret; +template <> struct DenseMapInfo { + static inline ExtentRef getEmptyKey() { return {}; } + static inline ExtentRef getTombstoneKey() { return {{Range(), Usr(-1)}}; } + static unsigned getHashValue(ExtentRef sym) { + return std::hash()(sym); } - static unsigned getHashValue(SymbolRef sym) { - return std::hash()(sym); - } - static bool isEqual(SymbolRef l, SymbolRef r) { return l == r; } + static bool isEqual(ExtentRef l, ExtentRef r) { return l == r; } }; } @@ -42,8 +38,8 @@ struct QueryFile { int id = -1; std::optional def; - llvm::DenseMap symbol2refcnt; - llvm::DenseMap outline2refcnt; + // `extent` is valid => declaration; invalid => regular reference + llvm::DenseMap symbol2refcnt; }; template struct QueryEntity {