[PATCH] D108194: [clangd] IncludeCleaner: Mark used headers

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Sorry, I thought i'd sent these comments...



================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:158
+
+std::vector<Inclusion>
+getUnused(IncludeStructure::HeaderID EntryPoint,
----------------
Why are we passing around Inclusions by value?


================
Comment at: clang-tools-extra/clangd/IncludeCleaner.h:51
+/// Retrieves IDs of all files containing SourceLocations from \p Locs.
+/// FIXME: Those locations could be within macro expansions and are resolved to
+/// their spelling/expansion locations.
----------------
This says FIXME but IIUC it's fixed.


================
Comment at: clang-tools-extra/clangd/IncludeCleaner.h:56
+
+inline llvm::DenseMap<IncludeStructure::HeaderID, bool> directlyReferencedFiles(
+    const IncludeStructure &Includes,
----------------
this function is undocumented, unused and untested :-)

What's it for? Why does it not return a set?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108194