[PATCH] D85426: [clangd] Implement FileFilter for the indexer

Kirill Bobyrev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 6 07:59:12 PDT 2020


kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:643
   assert(Loc.isValid() && "Invalid source location for NamedDecl");
-  // FIXME: use the result to filter out symbols.
-  shouldIndexFile(SM.getFileID(Loc));
+  if (!shouldIndexFile(SM.getFileID(Loc)))
+    return nullptr;
----------------
hokein wrote:
> A drive-by comment from D84811: the file granularity vs symbol granularity is tricky here.
> 
> Note that a *full* symbol (with declaration, definition, etc) may be formed from different files (.h, .cc), thinking of a following case:
> 
> ```
> // foo.h
> void func();
> 
> // user.cc
> #include "foo.h"
> 
> // foo.cc
> #include "foo.h"
> void func() {}
> ```
> 
> if our indexer indexes `user.cc` first, then `foo.h` is considered indexed, later when indexing `foo.cc`, we will skip the `func` symbol. so the symbol `foo` will not have definition.
>  
> 
Oh, you're right, good catch! That's why the post-filtering would probably work but maybe won't be as fancy :(

Thank you for mentioning it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85426



More information about the cfe-commits mailing list