[clang-tools-extra] r324735 - [clangd] Collect definitions when indexing.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 9 06:42:01 PST 2018
Author: sammccall
Date: Fri Feb 9 06:42:01 2018
New Revision: 324735
URL: http://llvm.org/viewvc/llvm-project?rev=324735&view=rev
Log:
[clangd] Collect definitions when indexing.
Within a TU:
- as now, collect a declaration from the first occurrence of a symbol
(taking clang's canonical declaration)
- when we first see a definition occurrence, copy the symbol and add it
Across TUs/sources:
- mergeSymbol in Merge.h is responsible for combining matching Symbols.
This covers dynamic/static merges and cross-TU merges in the static index.
- it prefers declarations from Symbols that have a definition.
- GlobalSymbolBuilderMain is modified to use mergeSymbol as a reduce step.
Random cleanups (can be pulled out):
- SymbolFromYAML -> SymbolsFromYAML, new singular SymbolFromYAML added
- avoid uninit'd SymbolLocations. Add an idiomatic way to check "absent".
- CanonicalDeclaration (as well as Definition) are mapped as optional in YAML.
- added operator<< for Symbol & SymbolLocation, for debugging
Reviewers: ioeric, hokein
Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits
Differential Revision: https://reviews.llvm.org/D42942
Modified:
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clang-tools-extra/trunk/clangd/index/Index.cpp
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/Merge.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
clang-tools-extra/trunk/clangd/index/SymbolYAML.h
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp Fri Feb 9 06:42:01 2018
@@ -14,9 +14,11 @@
//===---------------------------------------------------------------------===//
#include "index/Index.h"
+#include "index/Merge.h"
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
#include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/CompilerInstance.h"
#include "clang/Index/IndexDataConsumer.h"
#include "clang/Index/IndexingAction.h"
#include "clang/Tooling/CommonOptionsParser.h"
@@ -34,6 +36,7 @@ using clang::clangd::SymbolSlab;
namespace clang {
namespace clangd {
+namespace {
static llvm::cl::opt<std::string> AssumedHeaderDir(
"assume-header-dir",
@@ -91,6 +94,25 @@ public:
tooling::ExecutionContext *Ctx;
};
+// Combine occurrences of the same symbol across translation units.
+SymbolSlab mergeSymbols(tooling::ToolResults *Results) {
+ SymbolSlab::Builder UniqueSymbols;
+ llvm::BumpPtrAllocator Arena;
+ Symbol::Details Scratch;
+ Results->forEachResult([&](llvm::StringRef Key, llvm::StringRef Value) {
+ Arena.Reset();
+ auto Sym = clang::clangd::SymbolFromYAML(Value, Arena);
+ clang::clangd::SymbolID ID;
+ Key >> ID;
+ if (const auto *Existing = UniqueSymbols.find(ID))
+ UniqueSymbols.insert(mergeSymbol(*Existing, Sym, &Scratch));
+ else
+ UniqueSymbols.insert(Sym);
+ });
+ return std::move(UniqueSymbols).build();
+}
+
+} // namespace
} // namespace clangd
} // namespace clang
@@ -115,6 +137,7 @@ int main(int argc, const char **argv) {
return 1;
}
+ // Map phase: emit symbols found in each translation unit.
auto Err = Executor->get()->execute(
llvm::make_unique<clang::clangd::SymbolIndexActionFactory>(
Executor->get()->getExecutionContext()));
@@ -122,22 +145,11 @@ int main(int argc, const char **argv) {
llvm::errs() << llvm::toString(std::move(Err)) << "\n";
}
- // Deduplicate the result by key and keep the longest value.
- // FIXME(ioeric): Merge occurrences, rather than just dropping all but one.
- // Definitions and forward declarations have the same key and may both have
- // information. Usage count will need to be aggregated across occurrences,
- // too.
- llvm::StringMap<llvm::StringRef> UniqueSymbols;
- Executor->get()->getToolResults()->forEachResult(
- [&UniqueSymbols](llvm::StringRef Key, llvm::StringRef Value) {
- auto Ret = UniqueSymbols.try_emplace(Key, Value);
- if (!Ret.second) {
- // If key already exists, keep the longest value.
- llvm::StringRef &V = Ret.first->second;
- V = V.size() < Value.size() ? Value : V;
- }
- });
- for (const auto &Sym : UniqueSymbols)
- llvm::outs() << Sym.second;
+ // Reduce phase: combine symbols using the ID as a key.
+ auto UniqueSymbols =
+ clang::clangd::mergeSymbols(Executor->get()->getToolResults());
+
+ // Output phase: emit YAML for result symbols.
+ SymbolsToYAML(UniqueSymbols, llvm::outs());
return 0;
}
Modified: clang-tools-extra/trunk/clangd/index/Index.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.cpp?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Index.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Index.cpp Fri Feb 9 06:42:01 2018
@@ -10,11 +10,18 @@
#include "Index.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/SHA1.h"
+#include "llvm/Support/raw_ostream.h"
namespace clang {
namespace clangd {
using namespace llvm;
+raw_ostream &operator<<(raw_ostream &OS, const SymbolLocation &L) {
+ if (!L)
+ return OS << "(none)";
+ return OS << L.FileURI << "[" << L.StartOffset << "-" << L.EndOffset << "]";
+}
+
SymbolID::SymbolID(StringRef USR)
: HashValue(SHA1::hash(arrayRefFromStringRef(USR))) {}
@@ -29,6 +36,10 @@ void operator>>(StringRef Str, SymbolID
std::copy(HexString.begin(), HexString.end(), ID.HashValue.begin());
}
+raw_ostream &operator<<(raw_ostream &OS, const Symbol &S) {
+ return OS << S.Scope << S.Name;
+}
+
SymbolSlab::const_iterator SymbolSlab::find(const SymbolID &ID) const {
auto It = std::lower_bound(Symbols.begin(), Symbols.end(), ID,
[](const Symbol &S, const SymbolID &I) {
@@ -55,6 +66,7 @@ static void own(Symbol &S, DenseSet<Stri
Intern(S.Name);
Intern(S.Scope);
Intern(S.CanonicalDeclaration.FileURI);
+ Intern(S.Definition.FileURI);
Intern(S.CompletionLabel);
Intern(S.CompletionFilterText);
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Fri Feb 9 06:42:01 2018
@@ -27,11 +27,14 @@ struct SymbolLocation {
llvm::StringRef FileURI;
// The 0-based offset to the first character of the symbol from the beginning
// of the source file.
- unsigned StartOffset;
+ unsigned StartOffset = 0;
// The 0-based offset to the last character of the symbol from the beginning
// of the source file.
- unsigned EndOffset;
+ unsigned EndOffset = 0;
+
+ operator bool() const { return !FileURI.empty(); }
};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);
// The class identifies a particular C++ symbol (class, function, method, etc).
//
@@ -44,7 +47,7 @@ struct SymbolLocation {
class SymbolID {
public:
SymbolID() = default;
- SymbolID(llvm::StringRef USR);
+ explicit SymbolID(llvm::StringRef USR);
bool operator==(const SymbolID &Sym) const {
return HashValue == Sym.HashValue;
@@ -117,13 +120,16 @@ struct Symbol {
llvm::StringRef Name;
// The containing namespace. e.g. "" (global), "ns::" (top-level namespace).
llvm::StringRef Scope;
- // The location of the canonical declaration of the symbol.
+ // The location of the symbol's definition, if one was found.
+ // This covers the whole definition (e.g. class body).
+ SymbolLocation Definition;
+ // The location of the preferred declaration of the symbol.
+ // This may be the same as Definition.
//
- // A C++ symbol could have multiple declarations and one definition (e.g.
- // a function is declared in ".h" file, and is defined in ".cc" file).
- // * For classes, the canonical declaration is usually definition.
- // * For non-inline functions, the canonical declaration is a declaration
- // (not a definition), which is usually declared in ".h" file.
+ // A C++ symbol may have multiple declarations, and we pick one to prefer.
+ // * For classes, the canonical declaration should be the definition.
+ // * For non-inline functions, the canonical declaration typically appears
+ // in the ".h" file corresponding to the definition.
SymbolLocation CanonicalDeclaration;
/// A brief description of the symbol that can be displayed in the completion
@@ -160,12 +166,14 @@ struct Symbol {
// FIXME: add all occurrences support.
// FIXME: add extra fields for index scoring signals.
};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S);
// An immutable symbol container that stores a set of symbols.
// The container will maintain the lifetime of the symbols.
class SymbolSlab {
public:
using const_iterator = std::vector<Symbol>::const_iterator;
+ using iterator = const_iterator;
SymbolSlab() = default;
Modified: clang-tools-extra/trunk/clangd/index/Merge.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Merge.cpp?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Merge.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/Merge.cpp Fri Feb 9 06:42:01 2018
@@ -60,32 +60,40 @@ private:
Symbol
mergeSymbol(const Symbol &L, const Symbol &R, Symbol::Details *Scratch) {
assert(L.ID == R.ID);
- Symbol S = L;
- // For each optional field, fill it from R if missing in L.
- // (It might be missing in R too, but that's a no-op).
- if (S.CanonicalDeclaration.FileURI == "")
- S.CanonicalDeclaration = R.CanonicalDeclaration;
+ // We prefer information from TUs that saw the definition.
+ // Classes: this is the def itself. Functions: hopefully the header decl.
+ // If both did (or both didn't), continue to prefer L over R.
+ bool PreferR = R.Definition && !L.Definition;
+ Symbol S = PreferR ? R : L; // The target symbol we're merging into.
+ const Symbol &O = PreferR ? L : R; // The "other" less-preferred symbol.
+
+ // For each optional field, fill it from O if missing in S.
+ // (It might be missing in O too, but that's a no-op).
+ if (!S.Definition)
+ S.Definition = O.Definition;
+ if (!S.CanonicalDeclaration)
+ S.CanonicalDeclaration = O.CanonicalDeclaration;
if (S.CompletionLabel == "")
- S.CompletionLabel = R.CompletionLabel;
+ S.CompletionLabel = O.CompletionLabel;
if (S.CompletionFilterText == "")
- S.CompletionFilterText = R.CompletionFilterText;
+ S.CompletionFilterText = O.CompletionFilterText;
if (S.CompletionPlainInsertText == "")
- S.CompletionPlainInsertText = R.CompletionPlainInsertText;
+ S.CompletionPlainInsertText = O.CompletionPlainInsertText;
if (S.CompletionSnippetInsertText == "")
- S.CompletionSnippetInsertText = R.CompletionSnippetInsertText;
+ S.CompletionSnippetInsertText = O.CompletionSnippetInsertText;
- if (L.Detail && R.Detail) {
- // Copy into scratch space so we can merge.
- *Scratch = *L.Detail;
- if (Scratch->Documentation == "")
- Scratch->Documentation = R.Detail->Documentation;
- if (Scratch->CompletionDetail == "")
- Scratch->CompletionDetail = R.Detail->CompletionDetail;
- S.Detail = Scratch;
- } else if (L.Detail)
- S.Detail = L.Detail;
- else if (R.Detail)
- S.Detail = R.Detail;
+ if (O.Detail) {
+ if (S.Detail) {
+ // Copy into scratch space so we can merge.
+ *Scratch = *S.Detail;
+ if (Scratch->Documentation == "")
+ Scratch->Documentation = O.Detail->Documentation;
+ if (Scratch->CompletionDetail == "")
+ Scratch->CompletionDetail = O.Detail->CompletionDetail;
+ S.Detail = Scratch;
+ } else
+ S.Detail = O.Detail;
+ }
return S;
}
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Fri Feb 9 06:42:01 2018
@@ -132,13 +132,19 @@ bool shouldFilterDecl(const NamedDecl *N
// For symbols defined inside macros:
// * use expansion location, if the symbol is formed via macro concatenation.
// * use spelling location, otherwise.
+//
+// FIXME: EndOffset is inclusive (closed range), and should be exclusive.
+// FIXME: Because the underlying ranges are token ranges, this code chops the
+// last token in half if it contains multiple characters.
+// FIXME: We probably want to get just the location of the symbol name, not
+// the whole e.g. class.
llvm::Optional<SymbolLocation>
-getSymbolLocation(const NamedDecl *D, SourceManager &SM,
+getSymbolLocation(const NamedDecl &D, SourceManager &SM,
const SymbolCollector::Options &Opts,
std::string &FileURIStorage) {
- SourceLocation Loc = D->getLocation();
- SourceLocation StartLoc = SM.getSpellingLoc(D->getLocStart());
- SourceLocation EndLoc = SM.getSpellingLoc(D->getLocEnd());
+ SourceLocation Loc = D.getLocation();
+ SourceLocation StartLoc = SM.getSpellingLoc(D.getLocStart());
+ SourceLocation EndLoc = SM.getSpellingLoc(D.getLocEnd());
if (Loc.isMacroID()) {
std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
if (llvm::StringRef(PrintLoc).startswith("<scratch") ||
@@ -149,8 +155,8 @@ getSymbolLocation(const NamedDecl *D, So
// be "<scratch space>"
// * symbols controlled and defined by a compile command-line option
// `-DName=foo`, the spelling location will be "<command line>".
- StartLoc = SM.getExpansionRange(D->getLocStart()).first;
- EndLoc = SM.getExpansionRange(D->getLocEnd()).second;
+ StartLoc = SM.getExpansionRange(D.getLocStart()).first;
+ EndLoc = SM.getExpansionRange(D.getLocEnd()).second;
}
}
@@ -158,8 +164,11 @@ getSymbolLocation(const NamedDecl *D, So
if (!U)
return llvm::None;
FileURIStorage = std::move(*U);
- return SymbolLocation{FileURIStorage, SM.getFileOffset(StartLoc),
- SM.getFileOffset(EndLoc)};
+ SymbolLocation Result;
+ Result.FileURI = FileURIStorage;
+ Result.StartOffset = SM.getFileOffset(StartLoc);
+ Result.EndOffset = SM.getFileOffset(EndLoc);
+ return std::move(Result);
}
} // namespace
@@ -195,62 +204,86 @@ bool SymbolCollector::handleDeclOccurenc
return true;
auto ID = SymbolID(USR);
- if (Symbols.find(ID) != nullptr)
- return true;
+ const Symbol* BasicSymbol = Symbols.find(ID);
+ if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
+ BasicSymbol = addDeclaration(*ND, std::move(ID));
+ if (Roles & static_cast<unsigned>(index::SymbolRole::Definition))
+ addDefinition(*cast<NamedDecl>(ASTNode.OrigD), *BasicSymbol);
+ }
+ return true;
+}
- auto &SM = ND->getASTContext().getSourceManager();
+const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
+ SymbolID ID) {
+ auto &SM = ND.getASTContext().getSourceManager();
+
+ std::string QName;
+ llvm::raw_string_ostream OS(QName);
+ PrintingPolicy Policy(ASTCtx->getLangOpts());
+ // Note that inline namespaces are treated as transparent scopes. This
+ // reflects the way they're most commonly used for lookup. Ideally we'd
+ // include them, but at query time it's hard to find all the inline
+ // namespaces to query: the preamble doesn't have a dedicated list.
+ Policy.SuppressUnwrittenScope = true;
+ ND.printQualifiedName(OS, Policy);
+ OS.flush();
+
+ Symbol S;
+ S.ID = std::move(ID);
+ std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
+ S.SymInfo = index::getSymbolInfo(&ND);
+ std::string FileURI;
+ // FIXME: we may want a different "canonical" heuristic than clang chooses.
+ // Clang seems to choose the first, which may not have the most information.
+ if (auto DeclLoc = getSymbolLocation(ND, SM, Opts, FileURI))
+ S.CanonicalDeclaration = *DeclLoc;
- std::string QName;
- llvm::raw_string_ostream OS(QName);
- PrintingPolicy Policy(ASTCtx->getLangOpts());
- // Note that inline namespaces are treated as transparent scopes. This
- // reflects the way they're most commonly used for lookup. Ideally we'd
- // include them, but at query time it's hard to find all the inline
- // namespaces to query: the preamble doesn't have a dedicated list.
- Policy.SuppressUnwrittenScope = true;
- ND->printQualifiedName(OS, Policy);
- OS.flush();
-
- Symbol S;
- S.ID = std::move(ID);
- std::tie(S.Scope, S.Name) = splitQualifiedName(QName);
- S.SymInfo = index::getSymbolInfo(D);
- std::string URIStorage;
- if (auto DeclLoc = getSymbolLocation(ND, SM, Opts, URIStorage))
- S.CanonicalDeclaration = *DeclLoc;
-
- // Add completion info.
- assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
- CodeCompletionResult SymbolCompletion(ND, 0);
- const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
- *ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator,
- *CompletionTUInfo,
- /*IncludeBriefComments*/ true);
- std::string Label;
- std::string SnippetInsertText;
- std::string IgnoredLabel;
- std::string PlainInsertText;
- getLabelAndInsertText(*CCS, &Label, &SnippetInsertText,
- /*EnableSnippets=*/true);
- getLabelAndInsertText(*CCS, &IgnoredLabel, &PlainInsertText,
- /*EnableSnippets=*/false);
- std::string FilterText = getFilterText(*CCS);
- std::string Documentation = getDocumentation(*CCS);
- std::string CompletionDetail = getDetail(*CCS);
-
- S.CompletionFilterText = FilterText;
- S.CompletionLabel = Label;
- S.CompletionPlainInsertText = PlainInsertText;
- S.CompletionSnippetInsertText = SnippetInsertText;
- Symbol::Details Detail;
- Detail.Documentation = Documentation;
- Detail.CompletionDetail = CompletionDetail;
- S.Detail = &Detail;
+ // Add completion info.
+ // FIXME: we may want to choose a different redecl, or combine from several.
+ assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
+ CodeCompletionResult SymbolCompletion(&ND, 0);
+ const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
+ *ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator,
+ *CompletionTUInfo,
+ /*IncludeBriefComments*/ true);
+ std::string Label;
+ std::string SnippetInsertText;
+ std::string IgnoredLabel;
+ std::string PlainInsertText;
+ getLabelAndInsertText(*CCS, &Label, &SnippetInsertText,
+ /*EnableSnippets=*/true);
+ getLabelAndInsertText(*CCS, &IgnoredLabel, &PlainInsertText,
+ /*EnableSnippets=*/false);
+ std::string FilterText = getFilterText(*CCS);
+ std::string Documentation = getDocumentation(*CCS);
+ std::string CompletionDetail = getDetail(*CCS);
+
+ S.CompletionFilterText = FilterText;
+ S.CompletionLabel = Label;
+ S.CompletionPlainInsertText = PlainInsertText;
+ S.CompletionSnippetInsertText = SnippetInsertText;
+ Symbol::Details Detail;
+ Detail.Documentation = Documentation;
+ Detail.CompletionDetail = CompletionDetail;
+ S.Detail = &Detail;
- Symbols.insert(S);
- }
+ Symbols.insert(S);
+ return Symbols.find(S.ID);
+}
- return true;
+void SymbolCollector::addDefinition(const NamedDecl &ND,
+ const Symbol &DeclSym) {
+ if (DeclSym.Definition)
+ return;
+ // If we saw some forward declaration, we end up copying the symbol.
+ // This is not ideal, but avoids duplicating the "is this a definition" check
+ // in clang::index. We should only see one definition.
+ Symbol S = DeclSym;
+ std::string FileURI;
+ if (auto DefLoc = getSymbolLocation(ND, ND.getASTContext().getSourceManager(),
+ Opts, FileURI))
+ S.Definition = *DefLoc;
+ Symbols.insert(S);
}
} // namespace clangd
Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Fri Feb 9 06:42:01 2018
@@ -58,6 +58,9 @@ public:
SymbolSlab takeSymbols() { return std::move(Symbols).build(); }
private:
+ const Symbol *addDeclaration(const NamedDecl &, SymbolID);
+ void addDefinition(const NamedDecl &, const Symbol &DeclSymbol);
+
// All Symbols collected from the AST.
SymbolSlab::Builder Symbols;
ASTContext *ASTCtx;
Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp Fri Feb 9 06:42:01 2018
@@ -97,7 +97,9 @@ template <> struct MappingTraits<Symbol>
IO.mapRequired("Name", Sym.Name);
IO.mapRequired("Scope", Sym.Scope);
IO.mapRequired("SymInfo", Sym.SymInfo);
- IO.mapRequired("CanonicalDeclaration", Sym.CanonicalDeclaration);
+ IO.mapOptional("CanonicalDeclaration", Sym.CanonicalDeclaration,
+ SymbolLocation());
+ IO.mapOptional("Definition", Sym.Definition, SymbolLocation());
IO.mapRequired("CompletionLabel", Sym.CompletionLabel);
IO.mapRequired("CompletionFilterText", Sym.CompletionFilterText);
IO.mapRequired("CompletionPlainInsertText", Sym.CompletionPlainInsertText);
@@ -160,7 +162,7 @@ template <> struct ScalarEnumerationTrai
namespace clang {
namespace clangd {
-SymbolSlab SymbolFromYAML(llvm::StringRef YAMLContent) {
+SymbolSlab SymbolsFromYAML(llvm::StringRef YAMLContent) {
// Store data of pointer fields (excl. `StringRef`) like `Detail`.
llvm::BumpPtrAllocator Arena;
llvm::yaml::Input Yin(YAMLContent, &Arena);
@@ -173,13 +175,18 @@ SymbolSlab SymbolFromYAML(llvm::StringRe
return std::move(Syms).build();
}
-std::string SymbolsToYAML(const SymbolSlab& Symbols) {
- std::string Str;
- llvm::raw_string_ostream OS(Str);
+Symbol SymbolFromYAML(llvm::StringRef YAMLContent,
+ llvm::BumpPtrAllocator &Arena) {
+ llvm::yaml::Input Yin(YAMLContent, &Arena);
+ Symbol S;
+ Yin >> S;
+ return S;
+}
+
+void SymbolsToYAML(const SymbolSlab& Symbols, llvm::raw_ostream &OS) {
llvm::yaml::Output Yout(OS);
for (Symbol S : Symbols) // copy: Yout<< requires mutability.
Yout << S;
- return OS.str();
}
std::string SymbolToYAML(Symbol Sym) {
Modified: clang-tools-extra/trunk/clangd/index/SymbolYAML.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolYAML.h?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolYAML.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolYAML.h Fri Feb 9 06:42:01 2018
@@ -20,12 +20,17 @@
#include "Index.h"
#include "llvm/Support/Error.h"
+#include "llvm/Support/raw_ostream.h"
namespace clang {
namespace clangd {
// Read symbols from a YAML-format string.
-SymbolSlab SymbolFromYAML(llvm::StringRef YAMLContent);
+SymbolSlab SymbolsFromYAML(llvm::StringRef YAMLContent);
+
+// Read one symbol from a YAML-format string, backed by the arena.
+Symbol SymbolFromYAML(llvm::StringRef YAMLContent,
+ llvm::BumpPtrAllocator &Arena);
// Convert a single symbol to YAML-format string.
// The YAML result is safe to concatenate.
@@ -33,7 +38,7 @@ std::string SymbolToYAML(Symbol Sym);
// Convert symbols to a YAML-format string.
// The YAML result is safe to concatenate if you have multiple symbol slabs.
-std::string SymbolsToYAML(const SymbolSlab& Symbols);
+void SymbolsToYAML(const SymbolSlab& Symbols, llvm::raw_ostream &OS);
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Fri Feb 9 06:42:01 2018
@@ -37,7 +37,7 @@ std::unique_ptr<SymbolIndex> BuildStatic
llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
return nullptr;
}
- auto Slab = SymbolFromYAML(Buffer.get()->getBuffer());
+ auto Slab = SymbolsFromYAML(Buffer.get()->getBuffer());
SymbolSlab::Builder SymsBuilder;
for (auto Sym : Slab)
SymsBuilder.insert(Sym);
Modified: clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/IndexTests.cpp Fri Feb 9 06:42:01 2018
@@ -248,6 +248,28 @@ TEST(MergeTest, Merge) {
EXPECT_EQ(M.Detail->Documentation, "--doc--");
}
+TEST(MergeTest, PreferSymbolWithDefn) {
+ Symbol L, R;
+ Symbol::Details Scratch;
+
+ L.ID = R.ID = SymbolID("hello");
+ L.CanonicalDeclaration.FileURI = "file:/left.h";
+ R.CanonicalDeclaration.FileURI = "file:/right.h";
+ L.CompletionPlainInsertText = "left-insert";
+ R.CompletionPlainInsertText = "right-insert";
+
+ Symbol M = mergeSymbol(L, R, &Scratch);
+ EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/left.h");
+ EXPECT_EQ(M.Definition.FileURI, "");
+ EXPECT_EQ(M.CompletionPlainInsertText, "left-insert");
+
+ R.Definition.FileURI = "file:/right.cpp"; // Now right will be favored.
+ M = mergeSymbol(L, R, &Scratch);
+ EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/right.h");
+ EXPECT_EQ(M.Definition.FileURI, "file:/right.cpp");
+ EXPECT_EQ(M.CompletionPlainInsertText, "right-insert");
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=324735&r1=324734&r2=324735&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Fri Feb 9 06:42:01 2018
@@ -47,12 +47,17 @@ MATCHER_P(Snippet, S, "") {
}
MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
-MATCHER_P(LocationOffsets, Offsets, "") {
+MATCHER_P(DeclRange, Offsets, "") {
// Offset range in SymbolLocation is [start, end] while in Clangd is [start,
// end).
+ // FIXME: SymbolLocation should be [start, end).
return arg.CanonicalDeclaration.StartOffset == Offsets.first &&
arg.CanonicalDeclaration.EndOffset == Offsets.second - 1;
}
+MATCHER_P(DefRange, Offsets, "") {
+ return arg.Definition.StartOffset == Offsets.first &&
+ arg.Definition.EndOffset == Offsets.second - 1;
+}
namespace clang {
namespace clangd {
@@ -97,7 +102,8 @@ public:
auto Factory = llvm::make_unique<SymbolIndexActionFactory>(CollectorOpts);
std::vector<std::string> Args = {"symbol_collector", "-fsyntax-only",
- "-std=c++11", TestFileName};
+ "-std=c++11", "-include",
+ TestHeaderName, TestFileName};
Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
tooling::ToolInvocation Invocation(
Args,
@@ -106,14 +112,8 @@ public:
InMemoryFileSystem->addFile(TestHeaderName, 0,
llvm::MemoryBuffer::getMemBuffer(HeaderCode));
-
- std::string Content = MainCode;
- if (!HeaderCode.empty())
- Content = (llvm::Twine("#include\"") +
- llvm::sys::path::filename(TestHeaderName) + "\"\n" + Content)
- .str();
InMemoryFileSystem->addFile(TestFileName, 0,
- llvm::MemoryBuffer::getMemBuffer(Content));
+ llvm::MemoryBuffer::getMemBuffer(MainCode));
Invocation.run();
Symbols = Factory->Collector->takeSymbols();
return true;
@@ -176,6 +176,42 @@ TEST_F(SymbolCollectorTest, CollectSymbo
QName("foo::bar::v2"), QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, Locations) {
+ // FIXME: the behavior here is bad: chopping tokens, including more than the
+ // ident range, using half-open ranges. See fixmes in getSymbolLocation().
+ CollectorOpts.IndexMainFiles = true;
+ Annotations Header(R"cpp(
+ // Declared in header, defined in main.
+ $xdecl[[extern int X]];
+ $clsdecl[[class C]]ls;
+ $printdecl[[void print()]];
+
+ // Declared in header, defined nowhere.
+ $zdecl[[extern int Z]];
+ )cpp");
+ Annotations Main(R"cpp(
+ $xdef[[int X = 4]]2;
+ $clsdef[[class Cls {}]];
+ $printdef[[void print() {}]]
+
+ // Declared/defined in main only.
+ $y[[int Y]];
+ )cpp");
+ runSymbolCollector(Header.code(), Main.code());
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("X"), DeclRange(Header.offsetRange("xdecl")),
+ DefRange(Main.offsetRange("xdef"))),
+ AllOf(QName("Cls"), DeclRange(Header.offsetRange("clsdecl")),
+ DefRange(Main.offsetRange("clsdef"))),
+ AllOf(QName("print"), DeclRange(Header.offsetRange("printdecl")),
+ DefRange(Main.offsetRange("printdef"))),
+ AllOf(QName("Z"), DeclRange(Header.offsetRange("zdecl"))),
+ AllOf(QName("Y"), DeclRange(Main.offsetRange("y")),
+ DefRange(Main.offsetRange("y")))));
+}
+
TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
CollectorOpts.IndexMainFiles = false;
runSymbolCollector("class Foo {};", /*Main=*/"");
@@ -280,10 +316,9 @@ TEST_F(SymbolCollectorTest, SymbolFormed
EXPECT_THAT(
Symbols,
UnorderedElementsAre(
- AllOf(QName("abc_Test"),
- LocationOffsets(Header.offsetRange("expansion")),
+ AllOf(QName("abc_Test"), DeclRange(Header.offsetRange("expansion")),
DeclURI(TestHeaderURI)),
- AllOf(QName("Test"), LocationOffsets(Header.offsetRange("spelling")),
+ AllOf(QName("Test"), DeclRange(Header.offsetRange("spelling")),
DeclURI(TestHeaderURI))));
}
@@ -302,13 +337,13 @@ TEST_F(SymbolCollectorTest, SymbolFormed
FF2();
)");
runSymbolCollector(/*Header=*/"", Main.code());
- EXPECT_THAT(Symbols, UnorderedElementsAre(
- AllOf(QName("abc_Test"),
- LocationOffsets(Main.offsetRange("expansion")),
- DeclURI(TestFileURI)),
- AllOf(QName("Test"),
- LocationOffsets(Main.offsetRange("spelling")),
- DeclURI(TestFileURI))));
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("abc_Test"), DeclRange(Main.offsetRange("expansion")),
+ DeclURI(TestFileURI)),
+ AllOf(QName("Test"), DeclRange(Main.offsetRange("spelling")),
+ DeclURI(TestFileURI))));
}
TEST_F(SymbolCollectorTest, SymbolFormedByCLI) {
@@ -322,10 +357,10 @@ TEST_F(SymbolCollectorTest, SymbolFormed
runSymbolCollector(Header.code(), /*Main=*/"",
/*ExtraArgs=*/{"-DNAME=name"});
- EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(
- QName("name"),
- LocationOffsets(Header.offsetRange("expansion")),
- DeclURI(TestHeaderURI))));
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(AllOf(
+ QName("name"), DeclRange(Header.offsetRange("expansion")),
+ DeclURI(TestHeaderURI))));
}
TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
@@ -503,19 +538,23 @@ CompletionSnippetInsertText: 'snippet
...
)";
- auto Symbols1 = SymbolFromYAML(YAML1);
+ auto Symbols1 = SymbolsFromYAML(YAML1);
EXPECT_THAT(Symbols1,
UnorderedElementsAre(AllOf(
QName("clang::Foo1"), Labeled("Foo1-label"), Doc("Foo doc"),
Detail("int"), DeclURI("file:///path/foo.h"))));
- auto Symbols2 = SymbolFromYAML(YAML2);
+ auto Symbols2 = SymbolsFromYAML(YAML2);
EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(
QName("clang::Foo2"), Labeled("Foo2-label"),
Not(HasDetail()), DeclURI("file:///path/bar.h"))));
- std::string ConcatenatedYAML =
- SymbolsToYAML(Symbols1) + SymbolsToYAML(Symbols2);
- auto ConcatenatedSymbols = SymbolFromYAML(ConcatenatedYAML);
+ std::string ConcatenatedYAML;
+ {
+ llvm::raw_string_ostream OS(ConcatenatedYAML);
+ SymbolsToYAML(Symbols1, OS);
+ SymbolsToYAML(Symbols2, OS);
+ }
+ auto ConcatenatedSymbols = SymbolsFromYAML(ConcatenatedYAML);
EXPECT_THAT(ConcatenatedSymbols,
UnorderedElementsAre(QName("clang::Foo1"),
QName("clang::Foo2")));
More information about the cfe-commits
mailing list