dgoldman added inline comments.
================ Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:326 + auto Path = FE->getName(); + auto It = CachePathToFrameworkSpelling.find(Path); + if (It != CachePathToFrameworkSpelling.end()) ---------------- sammccall wrote: > any reason not to also try_emplace here and reuse the iterator below? In case the splitFrameworkHeaderPath fails we shouldn't cache the failure. But it's unexpected and shouldn't happen in practice now that I've added the private headers support, so flipped it to remove the entry if it happens. ================ Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:334 + // them. Cache the failure as well so we won't do this again for the same + // header. + CachePathToFrameworkSpelling[Path] = ""; ---------------- sammccall wrote: > We're (deliberately?) bypassing the umbrella cache here, say why private > headers shouldn't be replaced by the umbrella header? > Presumably because they're not exported by it... Just went ahead and added proper support for privateheaders since it wasn't too much more work. They should have a separate umbrella header (or none), see https://clang.llvm.org/docs/Modules.html#private-module-map-files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117056/new/ https://reviews.llvm.org/D117056 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
