kbobyrev added inline comments.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:247
for (const Inclusion &MFI : Structure.MainFileIncludes) {
// FIXME: Skip includes that are not self-contained.
if (!MFI.HeaderID) {
----------------
sammccall wrote:
> this is fixed now (though I think it would be better addressed ~here)
That's what I had initially (you can see the previous version of the patch,
after which I sent "oh, wrong place" and changed it). I think my idea was that
this looks confusing from the API standpoint. Choosing to filter out
non-guarded headers looks like a diagnostics decision and it looked awkward to
me to do that here: what `getUnused` does is simply merge two data structures.
I think it's somewhat similar to the patch where you mentioned "parse, don't
validate" and it is a great argument: the `<built-in>` and `<scratch-space>`
are indeed not "headers" in a sensible meaning of the term but non
self-contained ones are.
I don't have a very strong opinion on this, so I put the filtering back here
but I'm curious if you agree with my thoughs.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112695/new/
https://reviews.llvm.org/D112695
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
