VitaNuo added a comment.
Thanks for the comments!
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/OrganizeImports.cpp:62
+ const auto &SM = Inputs.AST->getSourceManager();
+ std::set<std::string> Spellings;
+ for (const auto &Missing : Findings.MissingIncludes)
----------------
kadircet wrote:
> prefer `llvm::StringSet<>`
I think it should actually be `llvm::DenseSet<inlude_cleaner::Header>`. We
should first de-duplicate providers and only then spell the resulting headers.
In this version, we were potentially spelling the same provider multiple times.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/OrganizeImports.cpp:64-67
+ for (const auto &P : Missing.Providers)
+ Spellings.insert(include_cleaner::spellHeader(
+ {P, Inputs.AST->getPreprocessor().getHeaderSearchInfo(),
+ SM.getFileEntryForID(SM.getMainFileID())}));
----------------
kadircet wrote:
> this will insert all providers for a symbol (e.g. if we have both `foo.h` and
> `bar.h`, we'll insert both).
you're right, thanks
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153769/new/
https://reviews.llvm.org/D153769
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
