sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:261
+ if (!FE) {
+ assert(isSpecialBuffer(FID, SM));
+ continue;
----------------
kbobyrev wrote:
> kbobyrev wrote:
> > Implementation-wise, this seems like the right place to drop nonexistent
> > "headers". However, this is a very surprising behavior from something like
> > "translateToHeaderIDs" for two reasons:
> >
> > - As the name states, it used to be a simple "translation" process, now
> > it's also filtering.
> > - `TranslatedHeaderIDs.size() == Files.size()` can be rather unexpected
> >
> > We never had 1:1 order etc translation assumptions but this is certainly
> > the new behavior we want to document.
> Thinking out loud: now I'm also thinking about the "handle non self-contained
> files" + "drop warnings for files without include warnings"
This function is mapping from a domain (FileIDs excluding macros) where special
buffers exist to a codomain (HeaderIDs) where they don't. Some sort of
filtering/handling is implied by the signature!
So I think it's not surprising at least if you think about it that way :-) But
this definitely needs to be documented, I'll do that.
> Thinking out loud: now I'm also thinking about the "handle non self-contained
> files" + "drop warnings for files without include warnings"
There may some small efficiency from mashing those into this loop as well, but
I don't think it's worth the confusion.
The reason I put this one inline instead of as a separate filtering step is
that the check we'd be doing is a natural part of the conversion. It's
basically a form of [parse, don't
validate](https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112652/new/
https://reviews.llvm.org/D112652
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
