kbobyrev added a comment.
Thank you for taking care of this!
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:261
+ if (!FE) {
+ assert(isSpecialBuffer(FID, SM));
+ continue;
----------------
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.
================
Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:261
+ if (!FE) {
+ assert(isSpecialBuffer(FID, SM));
+ continue;
----------------
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"
================
Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:275
+ SM.getPresumedLoc(SM.getLocForStartOfFile(FID)).getFilename());
+ // (Note we deduped the names as _number_ of <built-in>s is uninteresting).
+ EXPECT_THAT(ReferencedFileNames.keys(),
----------------
nit: Drop the parenthesis?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112652/new/
https://reviews.llvm.org/D112652
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
