r356446 - [Tooling] Add more scope specifiers until spelling is not ambiguous.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 19 03:12:16 PDT 2019
Author: ioeric
Date: Tue Mar 19 03:12:15 2019
New Revision: 356446
URL: http://llvm.org/viewvc/llvm-project?rev=356446&view=rev
Log:
[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
Modified:
cfe/trunk/lib/Tooling/Core/Lookup.cpp
cfe/trunk/unittests/Tooling/LookupTest.cpp
Modified: cfe/trunk/lib/Tooling/Core/Lookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Lookup.cpp?rev=356446&r1=356445&r2=356446&view=diff
==============================================================================
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp Tue Mar 19 03:12:15 2019
@@ -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 Neste
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<llvm::StringRef, 2> UnspelledScopes;
+ UnspelledSpecifier.split(UnspelledScopes, "::", /*MaxSplit=*/-1,
+ /*KeepEmpty=*/false);
- llvm::SmallVector<const NamespaceDecl *, 4> UseNamespaces =
+ llvm::SmallVector<const NamespaceDecl *, 4> 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(c
// 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);
}
Modified: cfe/trunk/unittests/Tooling/LookupTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/LookupTest.cpp?rev=356446&r1=356445&r2=356446&view=diff
==============================================================================
--- cfe/trunk/unittests/Tooling/LookupTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/LookupTest.cpp Tue Mar 19 03:12:15 2019
@@ -129,20 +129,38 @@ TEST(LookupTest, replaceNestedFunctionNa
// 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) {
More information about the cfe-commits
mailing list