From: Eric Liu Date: Tue, 19 Mar 2019 10:12:15 +0000 (+0000) Subject: [Tooling] Add more scope specifiers until spelling is not ambiguous. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=cdfada4895ab5c5e55e5e53e7d2a7aa9156d574a;p=clang [Tooling] Add more scope specifiers until spelling is not ambiguous. Summary: Previously, when the renamed spelling is ambiguous, we simply use the full-qualfied name (with leading "::"). This patch makes it try adding additional specifiers one at a time until name is no longer ambiguous, which allows us to find better disambuguated spelling. Reviewers: kadircet, gribozavr Subscribers: jdoerfert, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D59487 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@356446 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Tooling/Core/Lookup.cpp b/lib/Tooling/Core/Lookup.cpp index 0256438e80..6af533df5f 100644 --- a/lib/Tooling/Core/Lookup.cpp +++ b/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 @@ static bool isFullyQualified(const NestedNameSpecifier *NNS) { 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 @@ std::string tooling::replaceNestedName(const NestedNameSpecifier *Use, // 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); } diff --git a/unittests/Tooling/LookupTest.cpp b/unittests/Tooling/LookupTest.cpp index 530e1ef22a..2a4893e90c 100644 --- a/unittests/Tooling/LookupTest.cpp +++ b/unittests/Tooling/LookupTest.cpp @@ -129,20 +129,38 @@ TEST(LookupTest, replaceNestedFunctionName) { // 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) {