kadircet added a comment.
(pardon the interruption, some drive-by comments :))
================
Comment at: clang-tools-extra/clangd/Hover.cpp:1174
+ return;
+
+ for (const include_cleaner::Header &H : Providers) {
----------------
note that we don't care about each reference of a target, so speed things up
(rather than going through each reference inside the main file), you can check
if you've seen Ref.Target before and skip processing providers in that case.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:1175
+
+ for (const include_cleaner::Header &H : Providers) {
+ if (!ConvertedIncludes.match(H).empty()) {
----------------
note that this will attribute a symbol to it's non-preferred providers too,
e.g. if you have:
h1.h:
```
struct Foo; // defined in foo.h
struct Bar; // defined in bar.h
struct Baz; // defined in baz.h
struct H1 {};
```
I believe we want the following:
main.cc:
```
#include foo.h // Provides Foo
#include bar.h // Provides Bar
#include h1.h // Provides Baz, H1
Foo *x;
Bar *y;
Baz *z;
H1 *h;
```
and not saying that h1.h provides all Foo, Bar, Baz and H1 (otherwise an LLVM
header will always provide dozens of symbols, e.g.
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/Type.h#L109)
Basically in addition to checking that the particular header we're interested
in being a provider, we should also be checking there were no other providers
that are mentioned in the main file with a higher rank.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:1192
const SymbolIndex *Index) {
PrintingPolicy PP =
getPrintingPolicy(AST.getASTContext().getPrintingPolicy());
----------------
can you introduce a trace::metric here for tracking hover distributions on
certain symbol types? you can see an example in
https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/XRefs.cpp#L352
i believe the cases we care about are:
- include
- macro
- keyword
- expr
- decl
- attribute
(so that we have some idea about how often we provide this information)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146244/new/
https://reviews.llvm.org/D146244
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
