sammccall marked 4 inline comments as done.
sammccall added a comment.
PTAL
================
Comment at: clangd/ClangdServer.h:158
+ /// Retrieve locations for symbol references.
+ void references(PathRef File, Position Pos, bool includeDeclaration,
+ Callback<std::vector<Location>> CB);
----------------
ioeric wrote:
> ioeric wrote:
> > I think the C++ API can return `SymbolOccurrence` in the callback, which
> > would allow C++ API users to get richer information about the occurrences
> > e.g. kind, relationship, code snippet.
> nit: s/includeDeclaration/IncludeDeclaration/
I hadn't seen this comment, I agree, but the implementation that landed just
returns Locations. We can extend the C++ API but it doesn't belong in this
patch.
================
Comment at: clangd/XRefs.cpp:665
+std::vector<Location> references(ParsedAST &AST, Position Pos,
+ bool IncludeDeclaration,
+ const SymbolIndex *Index) {
----------------
hokein wrote:
> ilya-biryukov wrote:
> > Are we going to support the `IncludeDeclaration` option?
> > What should its semantics be?
> >
> > I'm trying to figure out a better name for it, can't get what should it do
> > by looking at the code at the moment :-)
> I think so. It is a term defined in LSP (although vscode always sets it
> `true`). I think it aligns with the `clang::index::SymbolRole::Declaration`.
It's not supported in the underlying implementation today, partly because we
weren't sure
about semantics (and usefulness).
FWIW I think it should control whether the roles `Declaration` *and*
`Definition` are included, because I guess it's meant to distinguish between
usages that introduce a name vs usages that reference an existing name.
For now, we don't support any options and don't parse them out of the LSP.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50896
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
