Skip to content

Commit

Permalink
[clangd] Fix AddUsing tweak for out-of-line functions.
Browse files Browse the repository at this point in the history
Summary:
We used getEnclosingNamespaceContext(), which calls getParent() rather
than getLexicalParent(), so we would end up adding the "using" line in
places that do not affect the cursor location, or just return an error
when declaration was in another file.

Patch by Adam Czachorowski!

Reviewers: hokein

Reviewed By: hokein

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D79496
  • Loading branch information
gislan authored and hokein committed May 7, 2020
1 parent 3bcd3dd commit 9108715
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 4 deletions.
10 changes: 6 additions & 4 deletions clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
Expand Up @@ -141,10 +141,12 @@ findInsertionPoint(const Tweak::Selection &Inputs,
}

// No relevant "using" statements. Try the nearest namespace level.
const auto *NS = Inputs.ASTSelection.commonAncestor()
->getDeclContext()
.getEnclosingNamespaceContext();
if (auto *ND = dyn_cast<NamespaceDecl>(NS)) {
const DeclContext *ParentDeclCtx =
&Inputs.ASTSelection.commonAncestor()->getDeclContext();
while (ParentDeclCtx && !ParentDeclCtx->isFileContext()) {
ParentDeclCtx = ParentDeclCtx->getLexicalParent();
}
if (auto *ND = llvm::dyn_cast_or_null<NamespaceDecl>(ParentDeclCtx)) {
auto Toks = Inputs.AST->getTokens().expandedTokens(ND->getSourceRange());
const auto *Tok = llvm::find_if(Toks, [](const syntax::Token &Tok) {
return Tok.kind() == tok::l_brace;
Expand Down
17 changes: 17 additions & 0 deletions clang-tools-extra/clangd/unittests/TweakTests.cpp
Expand Up @@ -2663,6 +2663,23 @@ using one::two::ff;
void fun() {
CALL(ff);
})cpp"},
// Parent namespace != lexical parent namespace
{R"cpp(
#include "test.hpp"
namespace foo { void fun(); }
void foo::fun() {
one::two::f^f();
})cpp",
R"cpp(
#include "test.hpp"
using one::two::ff;
namespace foo { void fun(); }
void foo::fun() {
ff();
})cpp"}};
llvm::StringMap<std::string> EditedFiles;
for (const auto &Case : Cases) {
Expand Down

0 comments on commit 9108715

Please sign in to comment.