kadircet added inline comments.

================
Comment at: clang-tools-extra/clangd/XRefs.h:52
+struct HoverInfo {
+  LocatedSymbol Sym;
+  /// Name of the context containing the symbol.
----------------
sammccall wrote:
> I'm not sure about reuse of LocatedSymbol - do we want to commit to returning 
> decl/def locations?
> Name might be enough here.
It might be nice to provide editors with enough info to jump to definition(it 
was brought up during last meeting). But happy to reduce it to just name.


================
Comment at: clang-tools-extra/clangd/XRefs.h:55
+  std::string ContainerName;
+  index::SymbolInfo SI;
+  /// Includes all the qualifiers.
----------------
sammccall wrote:
> SymbolInfo is a bit of a mess. Maybe we just want SymbolInfo::Kind? (LSP 
> SymbolKind is tempting but loses a lot of info for C++).
> 
> I do think we'll need to extend SymbolInfo::Kind or eventually use our own 
> enum, e.g. lambdas may need their own kind (they're variables, but don't have 
> a printable type and need to be displayed more like functions)
As you mentioned we might need to extend LSP's enum, but switching to it 
anyway. Currently it doesn't support "macro" kind exactly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61497/new/

https://reviews.llvm.org/D61497



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to