diff options
author | Eric Liu <ioeric@google.com> | 2018-10-22 12:48:49 +0000 |
---|---|---|
committer | Eric Liu <ioeric@google.com> | 2018-10-22 12:48:49 +0000 |
commit | ddad0f890f0830ce7389064b4a3a05d8ed7d6159 (patch) | |
tree | 20f51eb60db030b4545837b999213cc82945a753 /clang-tools-extra/change-namespace | |
parent | fe0112a68d6574c4b28181d946267cad6995762a (diff) |
[change-namespace] Enhance detection of conflicting namespaces.
Summary:
For example:
```
namespace util { class Base; }
namespace new {
namespace util { class Internal; }
}
namespace old {
util::Base b1;
}
```
When changing `old::` to `new::`, `util::` in namespace "new::" will conflict
with "new::util::" unless a leading "::" is added.
Reviewers: hokein
Subscribers: cfe-commits
Differential Revision: https://reviews.llvm.org/D53489
Diffstat (limited to 'clang-tools-extra/change-namespace')
-rw-r--r-- | clang-tools-extra/change-namespace/ChangeNamespace.cpp | 50 |
1 files changed, 39 insertions, 11 deletions
diff --git a/clang-tools-extra/change-namespace/ChangeNamespace.cpp b/clang-tools-extra/change-namespace/ChangeNamespace.cpp index f941e33d8d3..d0eaa306a79 100644 --- a/clang-tools-extra/change-namespace/ChangeNamespace.cpp +++ b/clang-tools-extra/change-namespace/ChangeNamespace.cpp @@ -7,8 +7,10 @@ // //===----------------------------------------------------------------------===// #include "ChangeNamespace.h" +#include "clang/AST/ASTContext.h" #include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" +#include "llvm/Support/Casting.h" #include "llvm/Support/ErrorHandling.h" using namespace clang::ast_matchers; @@ -283,23 +285,48 @@ bool isDeclVisibleAtLocation(const SourceManager &SM, const Decl *D, } // Given a qualified symbol name, returns true if the symbol will be -// incorrectly qualified without leading "::". -bool conflictInNamespace(llvm::StringRef QualifiedSymbol, +// incorrectly qualified without leading "::". For example, a symbol +// "nx::ny::Foo" in namespace "na::nx::ny" without leading "::"; a symbol +// "util::X" in namespace "na" can potentially conflict with "na::util" (if this +// exists). +bool conflictInNamespace(const ASTContext &AST, llvm::StringRef QualifiedSymbol, llvm::StringRef Namespace) { auto SymbolSplitted = splitSymbolName(QualifiedSymbol.trim(":")); assert(!SymbolSplitted.empty()); SymbolSplitted.pop_back(); // We are only interested in namespaces. - if (SymbolSplitted.size() > 1 && !Namespace.empty()) { + if (SymbolSplitted.size() >= 1 && !Namespace.empty()) { + auto SymbolTopNs = SymbolSplitted.front(); auto NsSplitted = splitSymbolName(Namespace.trim(":")); assert(!NsSplitted.empty()); - // We do not check the outermost namespace since it would not be a conflict - // if it equals to the symbol's outermost namespace and the symbol name - // would have been shortened. + + auto LookupDecl = [&AST](const Decl &Scope, + llvm::StringRef Name) -> const NamedDecl * { + const auto *DC = llvm::dyn_cast<DeclContext>(&Scope); + if (!DC) + return nullptr; + auto LookupRes = DC->lookup(DeclarationName(&AST.Idents.get(Name))); + if (LookupRes.empty()) + return nullptr; + return LookupRes.front(); + }; + // We do not check the outermost namespace since it would not be a + // conflict if it equals to the symbol's outermost namespace and the + // symbol name would have been shortened. + const NamedDecl *Scope = + LookupDecl(*AST.getTranslationUnitDecl(), NsSplitted.front()); for (auto I = NsSplitted.begin() + 1, E = NsSplitted.end(); I != E; ++I) { - if (*I == SymbolSplitted.front()) + if (*I == SymbolTopNs) // Handles "::ny" in "::nx::ny" case. return true; + // Handles "::util" and "::nx::util" conflicts. + if (Scope) { + if (LookupDecl(*Scope, SymbolTopNs)) + return true; + Scope = LookupDecl(*Scope, *I); + } } + if (Scope && LookupDecl(*Scope, SymbolTopNs)) + return true; } return false; } @@ -844,15 +871,16 @@ void ChangeNamespaceTool::replaceQualifiedSymbolInDeclContext( } } } + bool Conflict = conflictInNamespace(DeclCtx->getParentASTContext(), + ReplaceName, NewNamespace); // If the new nested name in the new namespace is the same as it was in the - // old namespace, we don't create replacement. - if (NestedName == ReplaceName || + // old namespace, we don't create replacement unless there can be ambiguity. + if ((NestedName == ReplaceName && !Conflict) || (NestedName.startswith("::") && NestedName.drop_front(2) == ReplaceName)) return; // If the reference need to be fully-qualified, add a leading "::" unless // NewNamespace is the global namespace. - if (ReplaceName == FromDeclName && !NewNamespace.empty() && - conflictInNamespace(ReplaceName, NewNamespace)) + if (ReplaceName == FromDeclName && !NewNamespace.empty() && Conflict) ReplaceName = "::" + ReplaceName; addReplacementOrDie(Start, End, ReplaceName, *Result.SourceManager, &FileToReplacements); |