ArcsinX added a comment. In D134379#3808043 <https://reviews.llvm.org/D134379#3808043>, @kadircet wrote:
> In D134379#3807770 <https://reviews.llvm.org/D134379#3807770>, @ArcsinX wrote: > >> Anyway if this is the only concern, we can handle namespace declaration as a >> special case inside `ReferencedLocationCrawler::add()`. something like this: >> >> diff >> - for (const Decl *Redecl : D->redecls()) >> - Result.User.insert(Redecl->getLocation()); >> + if (llvm::isa<NamespaceDecl>(D)) { >> + Result.User.insert(D->getCanonicalDecl()->getLocation()); >> + } else { >> + for (const Decl *Redecl : D->redecls()) >> + Result.User.insert(Redecl->getLocation()); >> + } >> >> And in the above example `#include bar.h` will be suggested to remove >> >> Could this be a suitable solution? > > I don't think picking the canonical declaration would be helpful in general > case; it'll likely preserve the first header all the time, which might be > unused otherwise. > > I feel like the best option here is, diagnosing the unused using directive as > well. but I am not sure if changing the lookup semantics would always be > "right". Diagnosing the unused using directive is a good option, but it's not related with `IncludeCleaner`. For now `IncludeCleaner` suggests to remove a header, but removing it leads to compile errors, which looks wrong. > The other option would be to consider headers in batches after full analysis > is complete, while keeping track of symbols provided by a particular header. > That way when we have a symbol that's only re-declared in a bunch of headers, > we can rank them based on other symbols being provided and only keep the > headers that are providing some extra "used" symbols in addition to redecls. > In the absence of such we get to pick an option between "all/none/forward > declare ourselves". I guess picking "all" in this case is less troublesome, > the user will get all the "unused include" warnings as soon as they use a > symbol from that namespace. This looks like a general problem, declaration of a used function in every header leads to "every header is used", maybe it's ok as far as it's not a common case. So, yes, with the proposed solution we will get rid of some "false positive" cases, but we will get some "false negative" cases. But IWYU handles this https://github.com/include-what-you-use/include-what-you-use/commit/97e236c55e5ebbd95b8bb221fd9497851f67c3cc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134379/new/ https://reviews.llvm.org/D134379 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
