[PATCH] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

No tests yet, posting for early feedback.



================
Comment at: clang-tools-extra/clangd/index/StdLib.cpp:233
   IndexOpts.StoreAllDocumentation = true;
+  // FIXME: Should we respect the Index.ReservedIdentifiers config here?
   // Sadly we can't use IndexOpts.FileFilter to restrict indexing scope.
----------------
I'm thinking **no**: the only time you'd want reserved identifiers from the standard library is if you're working on the standard library itself, but in that case the standard library source files would be part of the project and you'd be indexing them "normally", not via `indexStandardLibrary()` (so your configuration would be:

```
Index:
  ReservedIdentifiers: true
  StandardLibrary: false
```

and the `StandardLibrary: false` means this codepath is not taken).


================
Comment at: clang-tools-extra/clangd/indexer/IndexerMain.cpp:48
     Opts.CountReferences = true;
+    // FIXME: Do we have access to Config here? If so, should we respect
+    // Index.ReservedIdentifiers?
----------------
I'm thinking we should, so that projects that want to enable this (like e.g. the kernel) can use a remote index if desired.

I'm just not sure if we have the machinery in place to use Config from `clangd-indexer`.


================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:239
+  SymbolCollector::Options Options;
+  Options.CollectReserved = Config::current().Index.ReservedIdentifiers;
   if (!SymbolCollector::shouldCollectSymbol(
----------------
Would it be inappropriate to set this in the `SymbolCollector::Options` constructor, instead of every place where we call it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153946