ilya-biryukov added inline comments.
================
Comment at: clang-tools-extra/clangd/URI.h:115
+// the SourceManager.
+URI toURI(llvm::StringRef Path, const SourceManager &SM,
+ const std::string &FallbackDir);
----------------
This function does a very specialized form of path-to-uri conversion (fallback
dirs, traversing schemes, etc.)
I'm afraid that naming it `toURI` will mean everyone will find and use it
without giving it too much thought.
Another concern is layering. `URI.h` currently does not depend on
`SourceManager` and, arguably, it should stay that way. We define an interface
to extend the URI schemes that clangd supports, there's no reason for it to
depend on `SourceManager`.
================
Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:86
+ // Add macro references.
+ for (const auto &IDToRefs : MacroRefsToIndex->MacroRefs) {
+ for (const auto &Range : IDToRefs.second) {
----------------
This is trying to emulate existing logic in `SymbolCollector::finish`. Is there
a way we could share this?
Would avoid creating extra copies of reference slabs and allow to keep the code
in one place, rather than scattering it between `FileIndex.cpp` and
`SymbolCollector.cpp`. Would also allow to keep `toURI` private, meaning we
don't have to worry about naming it and the fact it's exposed in the public
interface.
One potential way to do this is to have an alternative version of
`handleMacroOccurence`, which would fill `SymbolCollector::MacroRefs` directly
and call this right after `indexTopLevelDecls`.
Would that work?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71406/new/
https://reviews.llvm.org/D71406
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
