Page MenuHomePhabricator

[clangd] XRef functions treat visible class definitions as primary.
Needs ReviewPublic

Authored by sammccall on Sep 15 2022, 2:27 PM.

Details

Reviewers
kadircet
Summary

This means go-to-type will take us to a class definition in most cases.
As an exception, if the class is declared in a header and defined in a CC file,
then these are treated as a decl/def pair.

This is (fairly) consistent with how the index behaves, though the index
benefits from aggregation across translation units so does a simpler "main file"
check rather than explicitly "is this a header".

Diff Detail

Event Timeline

sammccall created this revision.Sep 15 2022, 2:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 15 2022, 2:27 PM
Herald added a subscriber: arphaman. · View Herald Transcript
sammccall requested review of this revision.Sep 15 2022, 2:27 PM
kadircet added inline comments.Sep 16 2022, 2:57 AM
clang-tools-extra/clangd/XRefs.cpp
291

can we extract all of this into a const TagDecl* getPublicDecl(const TagDecl* TD) ?

303

nit: we actually have the main file name in ParsedAST, but it's probably not worth propagating that into here.

303

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?

nridge added a subscriber: nridge.Sep 18 2022, 12:52 AM