[PATCH] D117056: [clangd] Properly compute framework-style include spelling

David Goldman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 20 09:38:33 PST 2022

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

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list