Index: lib/Tooling/Core/Lookup.cpp =================================================================== --- lib/Tooling/Core/Lookup.cpp +++ lib/Tooling/Core/Lookup.cpp @@ -14,6 +14,7 @@ #include "clang/Tooling/Core/Lookup.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/DeclarationName.h" using namespace clang; using namespace clang::tooling; @@ -114,6 +115,37 @@ return false; } +// Returns true if spelling symbol \p QName as \p Spelling in \p UseContext is +// ambiguous. For example, if QName is "::y::bar" and the spelling is "y::bar" +// in `UseContext` "a" that contains a nested namespace "a::y", then "y::bar" +// can be resolved to ::a::y::bar, which can cause compile error. +static bool isAmbiguousNameInScope(StringRef Spelling, StringRef QName, + const DeclContext &UseContext) { + assert(QName.startswith("::")); + if (Spelling.startswith("::")) + return false; + + // Lookup the first component of Spelling in all enclosing namespaces and + // check if there is any existing symbols with the same name but in different + // scope. + StringRef Head = Spelling.split("::").first; + + llvm::SmallVector UseNamespaces = + getAllNamedNamespaces(&UseContext); + auto &AST = UseContext.getParentASTContext(); + StringRef TrimmedQName = QName.substr(2); + for (auto I = UseNamespaces.begin(), E = UseNamespaces.end(); I != E; ++I) { + const NamespaceDecl *NS = *I; + auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head))); + if (!LookupRes.empty()) { + for (const NamedDecl *Res : LookupRes) + if (!TrimmedQName.startswith(Res->getQualifiedNameAsString())) + return true; + } + } + return false; +} + std::string tooling::replaceNestedName(const NestedNameSpecifier *Use, const DeclContext *UseContext, const NamedDecl *FromDecl, @@ -146,6 +178,14 @@ // figure out how good a namespace match we have with our destination type. // We work backwards (from most specific possible namespace to least // specific). - return getBestNamespaceSubstr(UseContext, ReplacementString, - isFullyQualified(Use)); + StringRef Suggested = getBestNamespaceSubstr(UseContext, ReplacementString, + isFullyQualified(Use)); + // Use the fully qualified name if the suggested name is ambiguous. + // FIXME: consider re-shortening the name until the name is not ambiguous. We + // are not doing this because ambiguity is pretty bad and we should not try to + // be clever in handling such cases. Making this noticeable to users seems to + // be a better option. + return isAmbiguousNameInScope(Suggested, ReplacementString, *UseContext) + ? ReplacementString + : Suggested; } Index: unittests/Tooling/LookupTest.cpp =================================================================== --- unittests/Tooling/LookupTest.cpp +++ unittests/Tooling/LookupTest.cpp @@ -68,7 +68,7 @@ "namespace b { void f() { a::foo(); } }\n"); Visitor.OnCall = [&](CallExpr *Expr) { - EXPECT_EQ("a::bar", replaceCallExpr(Expr, "::a::bar")); + EXPECT_EQ("::a::bar", replaceCallExpr(Expr, "::a::bar")); }; Visitor.runOver("namespace a { void foo(); }\n" "namespace b { namespace a { void foo(); }\n" @@ -127,6 +127,38 @@ "namespace a { namespace b { namespace c {" "void f() { foo(); }" "} } }\n"); + + // If the shortest name is ambiguous, we need to add more qualifiers. + Visitor.OnCall = [&](CallExpr *Expr) { + EXPECT_EQ("::a::y::bar", replaceCallExpr(Expr, "::a::y::bar")); + }; + Visitor.runOver(R"( + namespace a { + namespace b { + namespace x { void foo() {} } + namespace y { void foo() {} } + } + } + + namespace a { + namespace b { + void f() { x::foo(); } + } + })"); + + Visitor.OnCall = [&](CallExpr *Expr) { + EXPECT_EQ("y::bar", replaceCallExpr(Expr, "::y::bar")); + }; + Visitor.runOver(R"( + namespace a { + namespace b { + namespace x { void foo() {} } + namespace y { void foo() {} } + } + } + + void f() { a::b::x::foo(); } + )"); } TEST(LookupTest, replaceNestedClassName) {