kadircet added a comment.
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".
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 is not easy in the current implementation of clangd, as we preserve
symbols provided by headers, but the new include-cleaner library we're working
on is more accommodating for such logic. Hence I'd rather leave this
implementation as-is today, and see if such a behaviour would be desired in
general (not only for namespace-decls, but pretty much any declaration for
which we can only see "forward declarations" without a clear "canonical
declaration"
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
