[PATCH] D114370: [clangd] IncludeCleaner: Attribute symbols from non self-contained headers to their parents

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.


================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:262
+         ID != SM.getMainFileID() && FE &&
+         !isSelfContainedHeader(PP, ID, FE);) {
+      ID = SM.getFileID(SM.getIncludeLoc(ID));
----------------
sammccall wrote:
> it seems like we'd be better off storing the "is-self-contained" in the IncludeStructure and looking up the HeaderID here rather than asking the preprocessor. That way we rely on info that's better obtained at preamble build time.
I am slightly confused: we don't really have the `IncludeStructure` here and it is logically detached from the `IncludeStructure` processing. We'd still have to unroll the chain of FIDs in here, so the only difference would be querying `IncludeStructure` data for the cached `isSelfContainedHeader` value, is that right? Why is obtaining that info at preamble build time better?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114370