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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
