[PATCH] D152900: [clangd] Update symbol collector to use include-cleaner.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 23 04:37:23 PDT 2023


kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:846
+void SymbolCollector::setSymbolProviders(
+    const Symbol &S, const llvm::SmallVector<include_cleaner::Header> Headers) {
+  if (Opts.CollectIncludePath &&
----------------
kadircet wrote:
> what about merging this one and `setIncludeLocation` ?
> 
> e.g:
> ```
> void SymbolCollector::setIncludeLocation(const Symbol &IdxS, SourceLocation DefLoc, const include_cleaner::Symbol &Sym) {
> if (!Opts.CollectIncludePath ||
>       shouldCollectIncludePath(S.SymInfo.Kind) == Symbol::Invalid)
>   return;
> IncludeFiles[SID] = ...;
> SymbolProviders[SID] = headersForSymbol(...);
> }
> ```
also sorry for missing this so far, but i think we should limit this to first provider initially. as otherwise we'll have providers that just accidentally declare certain symbols and we don't want them to be inserted (unless they're the only provider and end up at the top) just because our ranking heuristics fail or something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152900



More information about the cfe-commits mailing list