usaxena95 added inline comments.
================ Comment at: clang-tools-extra/clangd/XRefs.cpp:951 if (const auto *Tok = TB.spelledTokenAt(L)) - References.push_back({*Tok, Roles, ID}); + References.push_back({*Tok, Roles, getSymbolID(D)}); } ---------------- kadircet wrote: > we're still calling getsymbolid here lots of times, for probably the same > symbol. > > as discussed, what about building a lookup table on construction from > canonicaldecl to symbolid and use that lookup table instead of calling > getsymbolid over and over here? I don't mind adding a lookup table for this but this is not huge, only O(#ref) as compared to previous O(size of TU). ================ Comment at: clang-tools-extra/clangd/XRefs.cpp:1255 + targetDecl(N->ASTNode, Relations, AST.getHeuristicResolver())) { + TargetDecls.insert(D); + } ---------------- kadircet wrote: > i mean directly inserting canonicaldecls here. This would make it messy to duplicate this on each end of the call. + This has potential of introducing bugs in future findRef calls if we don't pass the canonical decls. Or we would have to assert that we only get canonical decls. I think this is an implementation detail of findRefs and should not be visible outside its API for simplicity. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125675/new/ https://reviews.llvm.org/D125675 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits