diff options
author | Sam McCall <sam.mccall@gmail.com> | 2018-01-19 21:58:58 +0000 |
---|---|---|
committer | Sam McCall <sam.mccall@gmail.com> | 2018-01-19 21:58:58 +0000 |
commit | bbb9a63c876bc0281283c1caa2e144ff9524c449 (patch) | |
tree | 1ea233ada013a99b6d252e6f895b1075b007196c | |
parent | fb61b3bf4eccaf4c325b8c2997c24cdf6a051146 (diff) |
[clangd] Change index scope convention from "outer::inner" to "outer::inner::"
Global scope is "" (was "")
Top-level namespace scope is "ns::" (was "ns")
Nested namespace scope is "ns::ns::" (was "ns::ns")
This composes more naturally:
- qname = scope + name
- full scope = resolved scope + unresolved scope (D42073 was the trigger)
It removes a wart from the old way: "foo::" has one more separator than "".
Another alternative that has these properties is "::ns", but that lacks
the property that both the scope and the name are substrings of the
qname as produced by clang.
git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@322996 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | clangd/CodeComplete.cpp | 14 | ||||
-rw-r--r-- | clangd/index/Index.h | 14 | ||||
-rw-r--r-- | clangd/index/SymbolCollector.cpp | 8 | ||||
-rw-r--r-- | unittests/clangd/CodeCompleteTests.cpp | 4 | ||||
-rw-r--r-- | unittests/clangd/FileIndexTests.cpp | 9 | ||||
-rw-r--r-- | unittests/clangd/SymbolCollectorTests.cpp | 8 |
6 files changed, 24 insertions, 33 deletions
diff --git a/clangd/CodeComplete.cpp b/clangd/CodeComplete.cpp index 1a95b6ba..ab8a22d6 100644 --- a/clangd/CodeComplete.cpp +++ b/clangd/CodeComplete.cpp @@ -313,7 +313,7 @@ llvm::Optional<SymbolID> getSymbolID(const CodeCompletionResult &R) { /// completion (e.g. "ns::ab?"). struct SpecifiedScope { /// The scope specifier as written. For example, for completion "ns::ab?", the - /// written scope specifier is "ns". + /// written scope specifier is "ns::". Doesn't include leading "::". std::string Written; // If this scope specifier is recognized in Sema (e.g. as a namespace // context), this will be set to the fully qualfied name of the corresponding @@ -321,8 +321,7 @@ struct SpecifiedScope { std::string Resolved; llvm::StringRef forIndex() { - llvm::StringRef Chosen = Resolved.empty() ? Written : Resolved; - return Chosen.trim(':'); + return Resolved.empty() ? Written.ltrim('::') : Resolved; } }; @@ -631,15 +630,14 @@ SpecifiedScope getSpecifiedScope(Sema &S, const CXXScopeSpec &SS) { auto SpecifierRange = SS.getRange(); Info.Written = Lexer::getSourceText( CharSourceRange::getCharRange(SpecifierRange), SM, clang::LangOptions()); + if (!Info.Written.empty()) + Info.Written += "::"; // Sema excludes the trailing ::. if (SS.isValid()) { DeclContext *DC = S.computeDeclContext(SS); if (auto *NS = llvm::dyn_cast<NamespaceDecl>(DC)) { - Info.Resolved = NS->getQualifiedNameAsString(); + Info.Resolved = NS->getQualifiedNameAsString() + "::"; } else if (llvm::dyn_cast<TranslationUnitDecl>(DC) != nullptr) { - Info.Resolved = "::"; - // Sema does not include the suffix "::" in the range of SS, so we add - // it back here. - Info.Written = "::"; + Info.Resolved = ""; } } return Info; diff --git a/clangd/index/Index.h b/clangd/index/Index.h index a70d38b7..a3eb298f 100644 --- a/clangd/index/Index.h +++ b/clangd/index/Index.h @@ -114,10 +114,9 @@ struct Symbol { SymbolID ID; // The symbol information, like symbol kind. index::SymbolInfo SymInfo; - // The unqualified name of the symbol, e.g. "bar" (for "n1::n2::bar"). + // The unqualified name of the symbol, e.g. "bar" (for ns::bar). llvm::StringRef Name; - // The scope (e.g. namespace) of the symbol, e.g. "n1::n2" (for - // "n1::n2::bar"). + // The containing namespace. e.g. "" (global), "ns::" (top-level namespace). llvm::StringRef Scope; // The location of the canonical declaration of the symbol. // @@ -221,12 +220,11 @@ struct FuzzyFindRequest { /// un-qualified identifiers and should not contain qualifiers like "::". std::string Query; /// \brief If this is non-empty, symbols must be in at least one of the scopes - /// (e.g. namespaces) excluding nested scopes. For example, if a scope "xyz" - /// is provided, the matched symbols must be defined in scope "xyz" but not - /// "xyz::abc". + /// (e.g. namespaces) excluding nested scopes. For example, if a scope "xyz::" + /// is provided, the matched symbols must be defined in namespace xyz but not + /// namespace xyz::abc. /// - /// A scope must be fully qualified without leading or trailing "::" e.g. - /// "n1::n2". "" is interpreted as the global namespace, and "::" is invalid. + /// The global scope is "", a top level scope is "foo::", etc. std::vector<std::string> Scopes; /// \brief The number of top candidates to return. The index may choose to /// return more than this, e.g. if it doesn't know which candidates are best. diff --git a/clangd/index/SymbolCollector.cpp b/clangd/index/SymbolCollector.cpp index 92402c5e..e7fae173 100644 --- a/clangd/index/SymbolCollector.cpp +++ b/clangd/index/SymbolCollector.cpp @@ -56,14 +56,14 @@ std::string makeAbsolutePath(const SourceManager &SM, StringRef Path) { return AbsolutePath.str(); } -// "a::b::c", return {"a::b", "c"}. Scope is empty if it doesn't exist. +// "a::b::c", return {"a::b::", "c"}. Scope is empty if there's no qualifier. std::pair<llvm::StringRef, llvm::StringRef> splitQualifiedName(llvm::StringRef QName) { assert(!QName.startswith("::") && "Qualified names should not start with ::"); size_t Pos = QName.rfind("::"); if (Pos == llvm::StringRef::npos) return {StringRef(), QName}; - return {QName.substr(0, Pos), QName.substr(Pos + 2)}; + return {QName.substr(0, Pos + 2), QName.substr(Pos + 2)}; } bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx, @@ -147,12 +147,10 @@ bool SymbolCollector::handleDeclOccurence( SymbolLocation Location = {FilePath, SM.getFileOffset(D->getLocStart()), SM.getFileOffset(D->getLocEnd())}; std::string QName = ND->getQualifiedNameAsString(); - auto ScopeAndName = splitQualifiedName(QName); Symbol S; S.ID = std::move(ID); - S.Scope = ScopeAndName.first; - S.Name = ScopeAndName.second; + std::tie(S.Scope, S.Name) = splitQualifiedName(QName); S.SymInfo = index::getSymbolInfo(D); S.CanonicalDeclaration = Location; diff --git a/unittests/clangd/CodeCompleteTests.cpp b/unittests/clangd/CodeCompleteTests.cpp index 7eb7b137..40005509 100644 --- a/unittests/clangd/CodeCompleteTests.cpp +++ b/unittests/clangd/CodeCompleteTests.cpp @@ -155,8 +155,8 @@ Symbol sym(StringRef QName, index::SymbolKind Kind, StringRef USRFormat) { Sym.Scope = ""; } else { Sym.Name = QName.substr(Pos + 2); - Sym.Scope = QName.substr(0, Pos); - USR += "@N@" + replace(Sym.Scope, "::", "@N@"); // ns:: -> @N@ns + Sym.Scope = QName.substr(0, Pos + 2); + USR += "@N@" + replace(QName.substr(0, Pos), "::", "@N@"); // ns:: -> @N@ns } USR += Regex("^.*$").sub(USRFormat, Sym.Name); // e.g. func -> @F@func# Sym.ID = SymbolID(USR); diff --git a/unittests/clangd/FileIndexTests.cpp b/unittests/clangd/FileIndexTests.cpp index adae1f1b..85352bc9 100644 --- a/unittests/clangd/FileIndexTests.cpp +++ b/unittests/clangd/FileIndexTests.cpp @@ -78,8 +78,7 @@ std::vector<std::string> match(const SymbolIndex &I, std::vector<std::string> Matches; auto Ctx = Context::empty(); I.fuzzyFind(Ctx, Req, [&](const Symbol &Sym) { - Matches.push_back( - (Sym.Scope + (Sym.Scope.empty() ? "" : "::") + Sym.Name).str()); + Matches.push_back((Sym.Scope + Sym.Name).str()); }); return Matches; } @@ -110,7 +109,7 @@ TEST(FileIndexTest, IndexAST) { FuzzyFindRequest Req; Req.Query = ""; - Req.Scopes = {"ns"}; + Req.Scopes = {"ns::"}; EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X")); } @@ -139,7 +138,7 @@ TEST(FileIndexTest, IndexMultiASTAndDeduplicate) { FuzzyFindRequest Req; Req.Query = ""; - Req.Scopes = {"ns"}; + Req.Scopes = {"ns::"}; EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X", "ns::ff")); } @@ -152,7 +151,7 @@ TEST(FileIndexTest, RemoveAST) { FuzzyFindRequest Req; Req.Query = ""; - Req.Scopes = {"ns"}; + Req.Scopes = {"ns::"}; EXPECT_THAT(match(M, Req), UnorderedElementsAre("ns::f", "ns::X")); M.update(Ctx, "f1", nullptr); diff --git a/unittests/clangd/SymbolCollectorTests.cpp b/unittests/clangd/SymbolCollectorTests.cpp index 87932574..c17d5350 100644 --- a/unittests/clangd/SymbolCollectorTests.cpp +++ b/unittests/clangd/SymbolCollectorTests.cpp @@ -43,9 +43,7 @@ MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; } MATCHER_P(Snippet, S, "") { return arg.CompletionSnippetInsertText == S; } -MATCHER_P(QName, Name, "") { - return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name; -} +MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; } namespace clang { namespace clangd { @@ -291,7 +289,7 @@ TEST_F(SymbolCollectorTest, YAMLConversions) { --- ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF856 Name: 'Foo1' -Scope: 'clang' +Scope: 'clang::' SymInfo: Kind: Function Lang: Cpp @@ -311,7 +309,7 @@ Detail: --- ID: 057557CEBF6E6B2DD437FBF60CC58F352D1DF858 Name: 'Foo2' -Scope: 'clang' +Scope: 'clang::' SymInfo: Kind: Function Lang: Cpp |