-
Notifications
You must be signed in to change notification settings - Fork 15.9k
LSP 3.18 - Symbol Tags #167536
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
LSP 3.18 - Symbol Tags #167536
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
8d5dd8b to
60a56c9
Compare
60a56c9 to
64d5a36
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
53c9892 to
a5a8555
Compare
Added tags node in the expected JSON output.
|
Hi @kadircet, @llvm-beanz, I would be very happy to see you as a reviewer! |
HighCommander4
left a comment
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.
Thanks for the PR and sorry for the long turnaround time in responding.
I have two high-level pieces of feedback.
First, I think it would be nice to cut down on the amount of new "API" we're exposing in AST.h, as well as some of the code duplication between symbol tags and highlighting modifiers.
(To give an example of what code duplication I'm referring to: it's not obvious why the logic for assigning the Declaration tag checks for UnresolvedUsingValueDecl. This comment explains it nicely, but rather than asking for the comment to be duplicated, I'd rather avoid the check itself being duplicated in the first place.)
Here's a specific proposal:
- Establish a "packed" representation for symbol tags. This can be as simple as
using SymbolTags = uint32_t;, with the interpretation that if tagNis present, then the bit1 << Nis set in the value.- (The reason for having a packed representation at all is so that we don't impose the cost of allocating vectors onto all consumers including semantic highlighting.)
- Have the public API in
AST.hjust beSymbolTags computeSymbolTags(Decl*)or similar. - The
FindSymbolscode can callcomputeSymbolTags(), and unpack the result into avector<SymbolTag>.- (If we later have multiple requests return a
vector<SymbolTag>, it's fine to expose a function that returns avector<SymbolTag>inAST.has well.)
- (If we later have multiple requests return a
- The
SemanticHighlightingcode can also callcomputeSymbolTags(), and use a mapping fromSymbolTagtoHighlightingModifierto convert them to highlighting modifiers. (It's fine to do this only for the subset of modifiers which are also symbol tags. The remaining modifiers can be added individually.)
This way, the logic (including the UnresolvedUsingValueDecl check) can remain centralized in the implementation of computeSymbolTags().
How does that sound?
Second, regarding testing strategy: given how relatively hard to read lit tests are, we generally use lit tests as a basic "smoke test" that the protocol conversions are working fine, while putting interesting logic into unit tests.
In this case, I think it would be good to have some unit tests that exercise the new computeSymbolTags() API. They don't need to be extensive, I think it's sufficient to test on a small piece of code similar to the one that symbol-tags.test sends in its didOpen, that the computed tags for various symbols are the expected ones.
To make this as easy as possible, I would suggest using the existing DocumentSymbols test fixture (example test), with a new matcher added around here that checks the symbol tags. (This will exercise the computeSymbolTags() API indirectly via its use in the FindSymbols feature, but I think that's good enough.) Happy to go into more details about this if it's not clear what I'm suggesting.
| }; | ||
| llvm::json::Value toJSON(const CodeAction &); | ||
|
|
||
| enum class SymbolTag { |
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.
If I'm understanding this correctly, the list and order here is coming from microsoft/language-server-protocol#2003.
Can we link to that PR from a comment here? That makes it clear that we're implementing something that's not in the spec yet, and serves as a reminder for us to update the implementation if the spec ends up being different from the current list in the PR.
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.
Done
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'm not seeing the requested comment.
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.
Done
For example, if we later give
I was thinking doing it in this PR (as I see you're already doing in your draft update), to avoid the code duplication mentioned earlier. |
HighCommander4
left a comment
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 know the patch is a draft but I took a quick look and spotted a few things.)
| const auto SymbolTags = computeSymbolTags(ND); | ||
| std::vector<SymbolTag> Tags; | ||
|
|
||
| if (SymbolTags & toSymbolTagBitmask(SymbolTag::Deprecated)) |
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.
We should be able to do this with a loop.
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.
Done
|
|
||
|
|
||
| // Whether T is const in a loose sense - is a variable with this type readonly? | ||
| bool isConst(QualType T); |
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.
Part of my motivation in asking not to have these granular functions in AST.h, is to avoid having them in any header file. They should be able to live wherever the implementation that computes symbol tags lives, in this case FindSymbols.cpp.
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.
Done
| MATCHER_P(withKind, Kind, "") { return arg.kind == Kind; } | ||
| MATCHER_P(withDetail, Detail, "") { return arg.detail == Detail; } | ||
| MATCHER_P(symRange, Range, "") { return arg.range == Range; } | ||
| MATCHER_P(withSymbolTag, Tag, "") { return llvm::is_contained(arg.tags, Tag); } |
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.
Instead of doing a separate lookup for each tag (quadratic in the number of tags), we can check them in a single go with:
Matcher<DocumentSymbol>
withSymbolTags(std::initializer_list<SymbolTag> Tags) {
return Field(&DocumentSymbol::tags, ElementsAreArray(Tags));
}and then
withSymbolTags({SymbolTag::Abstract, SymbolTag::Definition})You can also use UnorderedElementsAreArray if you'd prefer.
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.
Done
… Declarations and Definitions.
Refactoring on getSymbolTags and computeSymbolTags. Improved SymbolTags test.
clang-tools-extra/clangd/XRefs.cpp
Outdated
| HI.name = printName(Ctx, ND); | ||
| // FIXME: Populate HI.detail the way we do in symbolToHierarchyItem? | ||
| HI.kind = SK; | ||
| HI.tags = getSymbolTags(ND); |
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.
Is this change intended to be here, or in #170103?
Likewise for the change to TypeHierarchyItem in Protocol.h.
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.
| }; | ||
| llvm::json::Value toJSON(const CodeAction &); | ||
|
|
||
| enum class SymbolTag { |
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'm not seeing the requested comment.
| // | ||
| // This is confusing, and maybe we should use another name, but because "static" | ||
| // is a standard LSP modifier, having one with that name has advantages. | ||
| bool isStatic(const Decl *D) { |
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.
Please put the helpers (i.e. anything not exposed in the header) in the anonymous namespace.
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.
Done.
|
|
||
| // Indicates whether declaration D is a unique definition (as opposed to a | ||
| // declaration). | ||
| bool isUniqueDefinition(const NamedDecl *Decl); |
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.
This doesn't need to be exposed in the header.
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.
Done.
| // Do not treat an UnresolvedUsingValueDecl as a declaration. | ||
| // It's more common to think of it as a reference to the | ||
| // underlying declaration. | ||
| Result |= toSymbolTagBitmask(SymbolTag::Declaration); |
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'm a bit unclear on the intended semantics of the Declaration tag, especially as the FindSymbols call site is passing IsDecl = true.
Is the intention that every document symbol is a Declaration (except UnresolvedUsingValueDecl which is a minor edge case)?
The wording in the PR -- specifically "Render a symbol as definition (in contrast to declaration)" -- makes me think perhaps Declaration should only be set for things that are not a Definition?
(I have some ideas about cleaning up the API to avoid the IsDecl parameter, but we first need to understand what the behaviour we're trying to implement is.)
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.
In SemanticHighlighting.cpp (main branch Line 1132) there is an if-statement that I have kept (otherwise breaking tests) in my work (without precise knowledge of the purpose of the condition)
if (R.IsDecl) {
// Do not treat an UnresolvedUsingValueDecl as a declaration.
// It's more common to think of it as a reference to the
// underlying declaration.
if (!isa<UnresolvedUsingValueDecl>(Decl))
Tok.addModifier(HighlightingModifier::Declaration);
if (isUniqueDefinition(Decl))
Tok.addModifier(HighlightingModifier::Definition);
}now the if-statement is located in computeSymbolTags
const auto SymbolTags = computeSymbolTags(*Decl, R.IsDecl);
and the lines
...
const auto SymbolTags = computeSymbolTags(*Decl, R.IsDecl);
...
if (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration))
Tok.addModifier(HighlightingModifier::Declaration);
if (SymbolTags & toSymbolTagBitmask(SymbolTag::Definition))
Tok.addModifier(HighlightingModifier::Definition);
...replacing the code block, mentioned above.
The wording in the PR -- specifically "Render a symbol as definition (in contrast to declaration)" -- makes me think perhaps Declaration should only be set for things that are not a Definition?
When we decide to either ... or, some tests will break.
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.
The wording in the PR -- specifically "Render a symbol as definition (in contrast to declaration)" -- makes me think perhaps Declaration should only be set for things that are not a Definition?
When we decide to either ... or, some tests will break.
I'm assuming you're referring to semantic highlighting tests.
I'm talking here about the expected behaviour of textDocument/documentSymbol.
Say you're a client like Eclipse, and you send a textDocument/documentSymbol request, e.g. for the purpose of populating an Outline view. Are there any cases (ignoring the edge case of UnresolvedUsingValueDecl) where you'd want a returned symbol not to have the Declaration symbol tag set?
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.
There are no such cases.
The code block below arises from the assumption that a definition is always also a declaration. A definition cannot exist without a declaration. I now think that my previous assumption is incorrect, especially in this context.
if (IsDecl && 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
// underlying declaration.
Result |= toSymbolTagBitmask(SymbolTag::Declaration);
if (IsDef)
Result |= toSymbolTagBitmask(SymbolTag::Definition);
}
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.
There are no such cases.
Ok.
(This makes me question a bit why the Declaration modifier exists at all -- surely the authors of the LSP proposal weren't thinking of using-declarations -- but perhaps that's something we can follow up on in microsoft/language-server-protocol#2003.)
So, in terms of implementation strategy, I would suggest the following:
- Remove the
IsDeclparameter fromcomputeSymbolTags()(keep the implementation as if it wastrue) - In
SemanticHighlighting.cpp, do:if (R.IsDecl && (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration))) Tok.addModifier(HighlightingModifier::Declaration);
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.
Done.
| // Iterate through SymbolTag enum values and collect any that are present in | ||
| // the bitmask. SymbolTag values are in the numeric range | ||
| // [Deprecated .. ReadOnly]. | ||
| constexpr unsigned MinTag = static_cast<unsigned>(SymbolTag::Deprecated); |
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.
It may be more robust to add two special enum members:
enum class SymbolTag {
Deprecated = 1,
...
ReadOnly = 20,
// Update as needed
FirstTag = Deprecated,
LastTag = ReadOnly
};
and use those here.
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.
Done.
| class SymbolIndex; | ||
|
|
||
| /// A bitmask type representing symbol tags. | ||
| using SymbolTags = uint32_t; |
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.
Let's static_assert(SymbolTag::LastTag < 32). If the number of tags ever grows beyond that, it will serve as a reminder to change this to uint64_t.
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.
Done.
| TEST(DocumentSymbolsTest, SymbolTags) { | ||
| TestTU TU; | ||
| Annotations Main(R"( | ||
| class A { |
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.
It would be nice to test at least the amount of code we have in symbol-tags.test, i.e.
class AbstractClass{\n public:\n virtual ~AbstractClass() = default;\n virtual void f1() = 0;\n void f2() const;\n protected: \n void f3(){}\n private: \n static void f4(){} \n }; void AbstractClass::f2() const {} \n class ImplClass final: public AbstractClass { \n public: \n void f1() final {}};"
}}}
If you don't want to have that full code in two places, I'd rather have the shorter code in the lit test and the longer code here.
(Rationale: if we make tweaks to the tag behaviour, we'll want to accompany them with tests, and they'll be more readable as unit tests.)
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.
Done.
Refactored unit and integration tests. Robust SymbolTag declaration. Static assert of the symbol tag bit-mask's size. And other minor improvements...
|
|
||
| const auto SymbolTags = computeSymbolTags(*Decl, R.IsDecl); | ||
|
|
||
| if (SymbolTags & toSymbolTagBitmask(SymbolTag::Deprecated)) |
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.
Could we do something like declare a map at the top level in getSemanticHighlightins:
llvm::DenseMap<SymbolTag, HighlightingModifier> TagToModifierMap {
{SymbolTag::Deprecated, HighlightingModifier::Deprecated},
// other mappings as appropriate
};and then loop through the map here?
(Declaration can be omitted from the map and handled explicitly due to the extra logic discussed in the other comment.)
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.
Done.
| // Do not treat an UnresolvedUsingValueDecl as a declaration. | ||
| // It's more common to think of it as a reference to the | ||
| // underlying declaration. | ||
| Result |= toSymbolTagBitmask(SymbolTag::Declaration); |
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.
There are no such cases.
Ok.
(This makes me question a bit why the Declaration modifier exists at all -- surely the authors of the LSP proposal weren't thinking of using-declarations -- but perhaps that's something we can follow up on in microsoft/language-server-protocol#2003.)
So, in terms of implementation strategy, I would suggest the following:
- Remove the
IsDeclparameter fromcomputeSymbolTags()(keep the implementation as if it wastrue) - In
SemanticHighlighting.cpp, do:if (R.IsDecl && (SymbolTags & toSymbolTagBitmask(SymbolTag::Declaration))) Tok.addModifier(HighlightingModifier::Declaration);
refactored computeSymbolTags - no IsDecl is needed, refactored getSemanticHighlightings - intro map for tags and modifiers,
HighCommander4
left a comment
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.
Thanks -- looking pretty good, just minor comments remaining.
| /// same order that they appear. | ||
| llvm::Expected<std::vector<DocumentSymbol>> getDocumentSymbols(ParsedAST &AST); | ||
|
|
||
| /// Returns the symbol tags for the given declaration. |
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.
Maybe add:
/// This is a wrapper around computeSymbolTags() which unpacks
/// the tags into a vector.
and move the declaration below computeSymbolTags().
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.
Done.
clang-tools-extra/clangd/AST.cpp
Outdated
|
|
||
| #include "AST.h" | ||
|
|
||
| #include "SemanticHighlighting.h" |
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.
nit: This include is not necessary any more.
I would suggest removing the changes to AST.cpp and call-hierarchy.test from this diff altogether.
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.
Done.
clang-tools-extra/clangd/Protocol.h
Outdated
| SymbolKind kind; | ||
|
|
||
| /// The symbol tags for this item. | ||
| std::vector<SymbolTag> tags; |
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.
Per previous discussion, this change to TypeHierarchyItem shouldn't be in this patch
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.
Done.
| namespace clangd { | ||
|
|
||
| namespace { | ||
| SymbolTags toSymbolTagBitmask(const SymbolTag ST) { |
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 it makes sense to expose this function in FindSymbols.h, given that SemanticHighlighting.cpp also uses it (and more generally, that it's needed to make practical use of the result of computeSymbolTags())
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.
Done.
|
|
||
| const auto SymbolTags = computeSymbolTags(*Decl); | ||
|
|
||
| static const llvm::DenseMap<SymbolTag, HighlightingModifier> |
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.
This code can be run concurrently from multiple threads, so I don't think we can make it static.
But maybe we can make it static thread_local? We have existing uses of that in clangd code so it should work.
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.
Done.
|
|
||
| #include "Protocol.h" | ||
|
|
||
| #include "clang/AST/TypeBase.h" |
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.
Changes to this file can be dropped from the patch as well.
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.
Done.
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.
They're still here.
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.
Done.
- obsolete includes, - public toSymbolTagBitmask, - removed tags from TypeHierarchyItem, - thread_local map in getSemanticHighlightings - minor improvements
HighCommander4
left a comment
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.
Thanks! LGTM with final nits (drop changes to AST.h and SemanticHighlighting.h).
| #include "index/Symbol.h" | ||
| #include "index/SymbolID.h" | ||
| #include "clang/AST/Decl.h" | ||
| #include "clang/AST/DeclObjC.h" |
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.
nit: This removal seems like an include-what-you-use violation, as types like ObjCMethodDecl are referenced in this patch.
Just drop this?
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.
Done.
|
|
||
| #include "Protocol.h" | ||
|
|
||
| #include "clang/AST/TypeBase.h" |
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.
They're still here.
- removed obsolete forward-decl and includes - restored include
|
|
||
| #include "Protocol.h" | ||
| #include "llvm/ADT/StringRef.h" | ||
| #include "llvm/Support/raw_ostream.h" |
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.
There's still a change to SemanticHighlighting.h here.
(I would have fixed this for you myself and merged, but it looks like "Maintainers are allowed to edit this pull request" is not checked, so I can't 🤷 )
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.
Done.
|
@ratzdi Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
…llvm#167536) Implements support for symbol tags proposed for LSP 3.18 in microsoft/language-server-protocol#2003, in the `documentSymbols` and `workspace/symbols` requests. Fixes clangd/clangd#2123. --------- Co-authored-by: chouzz <zhouhua258@outlook.com> Co-authored-by: Dimitri Ratz <dimitri.ratz@thinkdigital.cc>
…llvm#167536) Implements support for symbol tags proposed for LSP 3.18 in microsoft/language-server-protocol#2003, in the `documentSymbols` and `workspace/symbols` requests. Fixes clangd/clangd#2123. --------- Co-authored-by: chouzz <zhouhua258@outlook.com> Co-authored-by: Dimitri Ratz <dimitri.ratz@thinkdigital.cc>
Implementation of the issues microsoft/language-server-protocol#2003 and clangd/clangd#2123. Continuation of #113669.
Related Follow-up PR: #170103