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

Reply via email to