sammccall added a comment.
This would be a great fix to have! However I don't think the specific changes
to the RecursiveASTVisitor are correct.
I can see a couple of approaches:
1. prevent the RecursiveASTVisitor from traversing into uninteresting contexts,
and drop the Encloses check in VisitUsingDecl
2. explicitly gather the list of interesting contexts
option 1 is most similar to this patch, I believe TraverseNamespaceDecl needs
to call base::TraverseNamespaceDecl only if the NS either encloses the
selection DC or is anonymous.
option 2 would mean walking up the declcontext chain and grabbing each entry
and also getAnonymousNamespace() from TranslationUnitDecl and NamespaceDecl,
and changing our Encloses() tests to check membership in that set.
================
Comment at: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp:97
+ bool TraverseNamespaceDecl(NamespaceDecl *D) {
+ for (auto *Decl : D->decls()) {
----------------
This doesn't look like it can be right:
- there's no distinction on whether the namespace is anonymous or not
- you're calling VisitUsingDecl, but also Base::TraverseNamespaceDecl which
will visit the contained using decls again
this also suggests we need more testcases here: some negative ones verifying
that we don't look at UsingDecls that are inside named namespaces.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156441/new/
https://reviews.llvm.org/D156441
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
