-
Notifications
You must be signed in to change notification settings - Fork 18k
Symbol tags in SymbolInformation, WorkspaceSymbol, CallHierarchyItem and TypeHierarchyItem #170103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 29 commits
910a890
01a022e
c450d62
c297add
35ee021
89237d1
9eb72dc
0b1c3f6
7e11d69
007764f
f1fdea3
2c0475f
525a6ec
d6865cc
66529e5
fad6461
f99012b
317abd3
477dd75
765ef3e
949f510
88aa2a1
bdacf73
86a41ca
2aea848
c39d64b
d54d642
88b5800
0ffcf77
c1005bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,7 +13,10 @@ | |
| #include "Quality.h" | ||
| #include "SourceCode.h" | ||
| #include "index/Index.h" | ||
| #include "index/Symbol.h" | ||
| #include "index/SymbolLocation.h" | ||
| #include "support/Logger.h" | ||
| #include "clang/AST/Decl.h" | ||
| #include "clang/AST/DeclFriend.h" | ||
| #include "clang/AST/DeclTemplate.h" | ||
| #include "clang/Index/IndexSymbol.h" | ||
|
|
@@ -135,6 +138,42 @@ bool isFinal(const Decl *D) { | |
| return false; | ||
| } | ||
|
|
||
| // A method "overrides" if: | ||
| // 1. It overrides at least one method | ||
| // 2. At least one of the overridden methods is virtual (but NOT pure | ||
| // virtual) | ||
| bool isOverrides(const NamedDecl *ND) { | ||
| if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(ND)) { | ||
| if (MD->size_overridden_methods() == 0) | ||
| return false; | ||
|
|
||
| for (const auto *Overridden : MD->overridden_methods()) { | ||
| // Check if the overridden method is virtual but not pure virtual | ||
| if (Overridden->isVirtual() && !Overridden->isPureVirtual()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of checking
|
||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // A method "implements" pure virtual methods from base classes if: | ||
| // 1. It overrides at least one method | ||
| // 2. It is NOT itself pure virtual (i.e., it has a concrete implementation) | ||
| // 3. ALL overridden methods are pure virtual | ||
| bool isImplements(const NamedDecl *ND) { | ||
| if (const auto *MD = llvm::dyn_cast<CXXMethodDecl>(ND)) { | ||
| if (MD->size_overridden_methods() == 0 || MD->isPureVirtual()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. I think we don't need to check |
||
| return false; | ||
|
|
||
| for (const auto *Overridden : MD->overridden_methods()) { | ||
| if (!Overridden->isPureVirtual()) | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // Indicates whether declaration D is a unique definition (as opposed to a | ||
| // declaration). | ||
| bool isUniqueDefinition(const NamedDecl *Decl) { | ||
|
|
@@ -153,10 +192,55 @@ bool isUniqueDefinition(const NamedDecl *Decl) { | |
| isa<TemplateTemplateParmDecl>(Decl) || isa<ObjCCategoryDecl>(Decl) || | ||
| isa<ObjCImplDecl>(Decl); | ||
| } | ||
|
|
||
| // Filter symbol tags based on the presence of other tags and the kind of | ||
| // symbol. This is needed to avoid redundant tags, e.g. Overrides implies | ||
| // Virtual and Implements implies Overrides/Virtual. | ||
| SymbolTags filterSymbolTags(SymbolTags ST) { | ||
| const SymbolTags VirtualMask = toSymbolTagBitmask(SymbolTag::Virtual); | ||
| const SymbolTags OverridesMask = toSymbolTagBitmask(SymbolTag::Overrides); | ||
| const SymbolTags ImplementsMask = toSymbolTagBitmask(SymbolTag::Implements); | ||
| const SymbolTags AbstractMask = toSymbolTagBitmask(SymbolTag::Abstract); | ||
| const SymbolTags FinalMask = toSymbolTagBitmask(SymbolTag::Final); | ||
|
|
||
| const SymbolTags RemoveVirtualAndOverrides = VirtualMask | OverridesMask; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the name of this variable should be |
||
|
|
||
| // Implements implies both Overrides and Virtual. | ||
| if (ST & ImplementsMask) | ||
| ST &= ~RemoveVirtualAndOverrides; | ||
|
|
||
| // Final also suppresses both Virtual and Overrides in this model. | ||
| if (ST & FinalMask) | ||
| ST &= ~RemoveVirtualAndOverrides; | ||
|
|
||
| // Overrides or Abstract each imply Virtual. | ||
| if (ST & (OverridesMask | AbstractMask)) | ||
| ST &= ~VirtualMask; | ||
|
|
||
| return ST; | ||
| } | ||
|
|
||
| bool isCXXClassMethod(const clang::clangd::Symbol &S) { | ||
| using clang::index::SymbolKind; | ||
| using clang::index::SymbolLanguage; | ||
|
|
||
| if (S.SymInfo.Lang != SymbolLanguage::CXX) | ||
| return false; | ||
|
timon-ul marked this conversation as resolved.
|
||
|
|
||
| return llvm::is_contained({SymbolKind::InstanceMethod, | ||
| SymbolKind::StaticMethod, SymbolKind::Constructor, | ||
| SymbolKind::Destructor, | ||
| SymbolKind::ConversionFunction}, | ||
| S.SymInfo.Kind); | ||
| } | ||
|
|
||
| template <typename E> constexpr E enumIncrement(E Value) { | ||
| return static_cast<E>(static_cast<std::underlying_type_t<E>>(Value) + 1); | ||
| } | ||
| } // namespace | ||
|
|
||
| SymbolTags toSymbolTagBitmask(const SymbolTag ST) { | ||
| return (1 << static_cast<unsigned>(ST)); | ||
| return (1 << static_cast<uint32_t>(ST)); | ||
|
timon-ul marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| SymbolTags computeSymbolTags(const NamedDecl &ND) { | ||
|
|
@@ -178,9 +262,15 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) { | |
| if (isAbstract(&ND)) | ||
| Result |= toSymbolTagBitmask(SymbolTag::Abstract); | ||
|
|
||
| if (isOverrides(&ND)) | ||
|
timon-ul marked this conversation as resolved.
|
||
| Result |= toSymbolTagBitmask(SymbolTag::Overrides); | ||
|
|
||
| if (isFinal(&ND)) | ||
| Result |= toSymbolTagBitmask(SymbolTag::Final); | ||
|
|
||
| if (isImplements(&ND)) | ||
| Result |= toSymbolTagBitmask(SymbolTag::Implements); | ||
|
|
||
| if (not isa<UnresolvedUsingValueDecl>(ND)) { | ||
| // Do not treat an UnresolvedUsingValueDecl as a declaration. | ||
| // It's more common to think of it as a reference to the | ||
|
|
@@ -208,26 +298,58 @@ SymbolTags computeSymbolTags(const NamedDecl &ND) { | |
| return Result; | ||
| } | ||
|
|
||
| std::vector<SymbolTag> getSymbolTags(const NamedDecl &ND) { | ||
| const auto symbolTags = computeSymbolTags(ND); | ||
| std::vector<SymbolTag> expandTagBitmask(const SymbolTags STGS) { | ||
|
timon-ul marked this conversation as resolved.
|
||
| std::vector<SymbolTag> Tags; | ||
|
|
||
| if (symbolTags == 0) | ||
| if (STGS == 0) | ||
| return Tags; | ||
|
|
||
| // No filtering required since this function is only used for Symbols from the | ||
| // index, which have already been filtered in getSymbolTags(const NamedDecl | ||
| // &ND). | ||
|
|
||
|
ratzdi marked this conversation as resolved.
|
||
| // Iterate through SymbolTag enum values and collect any that are present in | ||
| // the bitmask. SymbolTag values are in the numeric range | ||
| // [FirstTag .. LastTag]. | ||
| constexpr unsigned MinTag = static_cast<unsigned>(SymbolTag::FirstTag); | ||
|
timon-ul marked this conversation as resolved.
|
||
| constexpr unsigned MaxTag = static_cast<unsigned>(SymbolTag::LastTag); | ||
| for (unsigned I = MinTag; I <= MaxTag; ++I) { | ||
| auto ST = static_cast<SymbolTag>(I); | ||
| if (symbolTags & toSymbolTagBitmask(ST)) | ||
| if (STGS & toSymbolTagBitmask(ST)) | ||
| Tags.push_back(ST); | ||
| } | ||
| return Tags; | ||
| } | ||
|
|
||
| std::vector<SymbolTag> getSymbolTags(const Symbol &S) { | ||
| const SymbolTags Tags = | ||
| isCXXClassMethod(S) ? filterSymbolTags(S.Tags) : S.Tags; | ||
| return expandTagBitmask(Tags); | ||
| } | ||
|
|
||
| std::vector<SymbolTag> getSymbolTags(const NamedDecl &ND) { | ||
| const auto STGS = computeSymbolTags(ND); | ||
| SymbolTags FilteredTags = STGS; | ||
| std::vector<SymbolTag> Tags; | ||
|
|
||
| if (STGS == 0) | ||
| return Tags; | ||
|
|
||
| // Apply specific filter to the symbol tags only on CXX class methods. | ||
| if (isa<CXXMethodDecl>(ND)) | ||
| FilteredTags = filterSymbolTags(STGS); | ||
|
|
||
| // Iterate through SymbolTag enum values and collect any that are present in | ||
| // the bitmask. SymbolTag values are in the numeric range | ||
| // [FirstTag .. LastTag]. | ||
| for (SymbolTag Tag = SymbolTag::FirstTag; Tag <= SymbolTag::LastTag; | ||
| Tag = enumIncrement(Tag)) { | ||
| if (FilteredTags & toSymbolTagBitmask(Tag)) | ||
| Tags.push_back(Tag); | ||
| } | ||
| return Tags; | ||
| } | ||
|
|
||
| namespace { | ||
| using ScoredSymbolInfo = std::pair<float, SymbolInformation>; | ||
| struct ScoredSymbolGreater { | ||
|
|
@@ -359,6 +481,7 @@ getWorkspaceSymbols(llvm::StringRef Query, int Limit, | |
| Info.score = Relevance.NameMatch > std::numeric_limits<float>::epsilon() | ||
| ? Score / Relevance.NameMatch | ||
| : QualScore; | ||
| Info.tags = getSymbolTags(Sym); | ||
| Top.push({Score, std::move(Info)}); | ||
| }); | ||
| for (auto &R : std::move(Top).items()) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to check
MD->size_overridden_methods()here, because at line 150 we have a loop which will be just no-op, if there are not overridden methods.