[PATCH] D144976: [clangd] Add provider info on symbol hover.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 14 01:58:56 PDT 2023


hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/Hover.cpp:1099
+  trace::Span Tracer("Hover::maybeAddSymbolProviders");
+  include_cleaner::walkUsed(
+      AST.getLocalTopLevelDecls(), MacroReferences, AST.getPragmaIncludes(),
----------------
It seems that the `walkUsed` API might be not the best fit. `walkUsed` API has some special logic on handling different AST nodes, e.g. refs of operators are ignored, so if we hover on an operator ref, we will not show the providing header (which we should).

Our goal is to provide the information (header) where the symbol under the hover comes from (ref satisfaction is out of the scope). I think `include_cleaner::headersForSymbol` is a better fit for our purpose, and the implementation is simpler:

- on hover, we have the selected symbol (either a regular declaration or a macro)
- it is easy to construct a `include_cleaner::Symbol` from the selected symbol
- choose the first result from `headersForSymbol`

To do that we need to expose `headersForSymbol` from the internal `AnalysisInternal.h`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D144976/new/

https://reviews.llvm.org/D144976



More information about the cfe-commits mailing list