Index: cfe/trunk/lib/Tooling/Core/Lookup.cpp =================================================================== --- cfe/trunk/lib/Tooling/Core/Lookup.cpp +++ cfe/trunk/lib/Tooling/Core/Lookup.cpp @@ -14,6 +14,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclarationName.h" +#include "llvm/ADT/SmallVector.h" using namespace clang; using namespace clang::tooling; @@ -114,35 +115,60 @@ 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. +// Adds more scope specifier to the spelled name until the spelling is not +// ambiguous. A spelling is ambiguous if the resolution of the symbol is +// ambiguous. For example, if QName is "::y::bar", the spelling is "y::bar", and +// context contains a nested namespace "a::y", then "y::bar" can be resolved to +// ::a::y::bar in the context, which can cause compile error. // FIXME: consider using namespaces. -static bool isAmbiguousNameInScope(StringRef Spelling, StringRef QName, - const DeclContext &UseContext) { +static std::string disambiguateSpellingInScope(StringRef Spelling, + StringRef QName, + const DeclContext &UseContext) { assert(QName.startswith("::")); + assert(QName.endswith(Spelling)); if (Spelling.startswith("::")) - return false; + return Spelling; - // 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; + auto UnspelledSpecifier = QName.drop_back(Spelling.size()); + llvm::SmallVector UnspelledScopes; + UnspelledSpecifier.split(UnspelledScopes, "::", /*MaxSplit=*/-1, + /*KeepEmpty=*/false); - llvm::SmallVector UseNamespaces = + llvm::SmallVector EnclosingNamespaces = getAllNamedNamespaces(&UseContext); auto &AST = UseContext.getParentASTContext(); StringRef TrimmedQName = QName.substr(2); - for (const auto *NS : UseNamespaces) { - auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head))); - if (!LookupRes.empty()) { - for (const NamedDecl *Res : LookupRes) - if (!TrimmedQName.startswith(Res->getQualifiedNameAsString())) - return true; + + auto IsAmbiguousSpelling = [&EnclosingNamespaces, &AST, &TrimmedQName]( + const llvm::StringRef CurSpelling) { + if (CurSpelling.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 = CurSpelling.split("::").first; + for (const auto *NS : EnclosingNamespaces) { + 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; + }; + + // Add more qualifiers until the spelling is not ambiguous. + std::string Disambiguated = Spelling; + while (IsAmbiguousSpelling(Disambiguated)) { + if (UnspelledScopes.empty()) { + Disambiguated = "::" + Disambiguated; + } else { + Disambiguated = (UnspelledScopes.back() + "::" + Disambiguated).str(); + UnspelledScopes.pop_back(); } } - return false; + return Disambiguated; } std::string tooling::replaceNestedName(const NestedNameSpecifier *Use, @@ -179,12 +205,6 @@ // specific). 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; + + return disambiguateSpellingInScope(Suggested, ReplacementString, *UseContext); } Index: cfe/trunk/unittests/Tooling/LookupTest.cpp =================================================================== --- cfe/trunk/unittests/Tooling/LookupTest.cpp +++ cfe/trunk/unittests/Tooling/LookupTest.cpp @@ -129,20 +129,38 @@ // 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")); + 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 b { + namespace x { void foo() {} } + namespace y { void foo() {} } + } } namespace a { - namespace b { - void f() { x::foo(); } + namespace b { + void f() { x::foo(); } + } + })"); + + Visitor.OnCall = [&](CallExpr *Expr) { + // y::bar would be ambiguous due to "a::b::y". + EXPECT_EQ("::y::bar", replaceCallExpr(Expr, "::y::bar")); + }; + Visitor.runOver(R"( + namespace a { + namespace b { + void foo() {} + namespace y { } + } } + + namespace a { + namespace b { + void f() { foo(); } + } })"); Visitor.OnCall = [&](CallExpr *Expr) {