[PATCH] D94477: [clangd] Add main file macros into the main-file index.

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 12 02:38:04 PST 2021


kadircet added a comment.

Ah this is really unfortunate :( In theory we already know which slabs belong to main file while updating the index for the preamble, as we shard them per file. But it is really inconvenient layering wise to transfer that slab from `updatePreamble` to `updateMain` in parsing callbacks :/
Another layer to fix the issue would be to somehow make main file parsing, re-parse those macro definitions through replay mechanism or preamble patching, but these might have unwanted consequences as we would see a re-definition of the macro now. Also they are considerably hard :/

So it feels like the solution proposed here isn't so bad after all (unless there are other alternatives i am missing ATM). I would suggest having a separate container for macro definitions though, rather than modifying the existing "references container". We could directly have SymbolSlab/vector<Symbol> for the definitions and merge them with SymbolCollector's existing slab during the post-processing. That way we can also re-use the logic in `SymbolCollector::handleMacroOccurence` to generate a `Symbol` from `MacroInfo`.

On a side note, we can also easily skip the shard belonging to main file inside FileIndex::updatePreamble, which is going to be suppressed by new merged-index logic anyway. Though I think it deserves a separate patch. Also I don't fully grasp the workings of the suppression logic yet, so it might not be wise to do so if there are any cases in which we still merge information from static index.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94477



More information about the cfe-commits mailing list