kadircet added inline comments.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:291
+ // (This exception matches the spirit of our indexing heuristics).
+ if (/*PreferDef=*/[&] {
+ if (Def == TD)
----------------
can we extract all of this into a `const TagDecl* getPublicDecl(const TagDecl*
TD)` ?
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:303
+ // decl is some other header.
+ return isHeaderFile(SM.getFileEntryRefForID(DefFile)->getName(),
+ D->getASTContext().getLangOpts());
----------------
nit: we actually have the main file name in ParsedAST, but it's probably not
worth propagating that into here.
================
Comment at: clang-tools-extra/clangd/XRefs.cpp:303
+ // decl is some other header.
+ return isHeaderFile(SM.getFileEntryRefForID(DefFile)->getName(),
+ D->getASTContext().getLangOpts());
----------------
kadircet wrote:
> nit: we actually have the main file name in ParsedAST, but it's probably not
> worth propagating that into here.
reaching here actually implies user is in the same file that has the type
definition (I know we were mostly discussing this for index behaviour, and our
reasoning for the index makes completely sense) but I am not sure if taking the
user to public declaration, when they're actually writing some code in the same
file as the type's private implementation is still the right thing to do.
I think; if the definition is visible through AST, we should always prefer
that, unless it's in a private header (which in theory we can detect here, as
we've information about include graph).
do you think jumping to public interface is still desirable even when the user
is already implementing something inside the private implementation file?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133979/new/
https://reviews.llvm.org/D133979
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
