Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -285,8 +285,7 @@ } llvm::Optional headerToInsertIfNotPresent() const { - if (!IndexResult || !IndexResult->Detail || - IndexResult->Detail->IncludeHeader.empty()) + if (!IndexResult || IndexResult->IncludeHeader.empty()) return llvm::None; if (SemaResult && SemaResult->Declaration) { // Avoid inserting new #include if the declaration is found in the current @@ -296,7 +295,7 @@ if (SM.isInMainFile(SM.getExpansionLoc(RD->getBeginLoc()))) return llvm::None; } - return IndexResult->Detail->IncludeHeader; + return IndexResult->IncludeHeader; } using Bundle = llvm::SmallVector; @@ -382,7 +381,7 @@ log("Failed to generate include insertion edits for adding header " "(FileURI='{0}', IncludeHeader='{1}') into {2}", C.IndexResult->CanonicalDeclaration.FileURI, - C.IndexResult->Detail->IncludeHeader, FileName); + C.IndexResult->IncludeHeader, FileName); } } @@ -397,12 +396,11 @@ } else if (C.IndexResult) { S.Signature = C.IndexResult->Signature; S.SnippetSuffix = C.IndexResult->CompletionSnippetSuffix; - if (auto *D = C.IndexResult->Detail) - S.ReturnType = D->ReturnType; + S.ReturnType = C.IndexResult->ReturnType; } if (ExtractDocumentation && Completion.Documentation.empty()) { - if (C.IndexResult && C.IndexResult->Detail) - Completion.Documentation = C.IndexResult->Detail->Documentation; + if (C.IndexResult) + Completion.Documentation = C.IndexResult->Documentation; else if (C.SemaResult) Completion.Documentation = getDocComment(ASTCtx, *C.SemaResult, /*CommentsFromHeader=*/false); @@ -846,9 +844,8 @@ IndexRequest.IDs.insert(*S.IDForDoc); } Index->lookup(IndexRequest, [&](const Symbol &S) { - if (!S.Detail || S.Detail->Documentation.empty()) - return; - FetchedDocs[S.ID] = S.Detail->Documentation; + if (!S.Documentation.empty()) + FetchedDocs[S.ID] = S.Documentation; }); log("SigHelp: requested docs for {0} symbols from the index, got {1} " "symbols with non-empty docs in the response", Index: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp =================================================================== --- clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp +++ clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp @@ -160,17 +160,14 @@ SymbolSlab mergeResults() override { SymbolSlab::Builder UniqueSymbols; - llvm::BumpPtrAllocator Arena; - Symbol::Details Scratch; Executor.getToolResults()->forEachResult( [&](llvm::StringRef Key, llvm::StringRef Value) { - Arena.Reset(); - llvm::yaml::Input Yin(Value, &Arena); - auto Sym = clang::clangd::SymbolFromYAML(Yin, Arena); + llvm::yaml::Input Yin(Value); + auto Sym = clang::clangd::SymbolFromYAML(Yin); clang::clangd::SymbolID ID; Key >> ID; if (const auto *Existing = UniqueSymbols.find(ID)) - UniqueSymbols.insert(mergeSymbol(*Existing, Sym, &Scratch)); + UniqueSymbols.insert(mergeSymbol(*Existing, Sym)); else UniqueSymbols.insert(Sym); }); @@ -188,9 +185,8 @@ void consumeSymbols(SymbolSlab Symbols) override { std::lock_guard Lock(Mut); for (auto &&Sym : Symbols) { - Symbol::Details Scratch; if (const auto *Existing = Result.find(Sym.ID)) - Result.insert(mergeSymbol(*Existing, Sym, &Scratch)); + Result.insert(mergeSymbol(*Existing, Sym)); else Result.insert(Sym); } Index: clangd/index/Index.h =================================================================== --- clangd/index/Index.h +++ clangd/index/Index.h @@ -199,30 +199,19 @@ /// This is in LSP snippet syntax (e.g. "({$0})" for a no-args function). /// (When snippets are disabled, the symbol name alone is used). llvm::StringRef CompletionSnippetSuffix; - - /// Optional symbol details that are not required to be set. For example, an - /// index fuzzy match can return a large number of symbol candidates, and it - /// is preferable to send only core symbol information in the batched results - /// and have clients resolve full symbol information for a specific candidate - /// if needed. - struct Details { - /// Documentation including comment for the symbol declaration. - llvm::StringRef Documentation; - /// Type when this symbol is used in an expression. (Short display form). - /// e.g. return type of a function, or type of a variable. - llvm::StringRef ReturnType; - /// This can be either a URI of the header to be #include'd for this symbol, - /// or a literal header quoted with <> or "" that is suitable to be included - /// directly. When this is a URI, the exact #include path needs to be - /// calculated according to the URI scheme. - /// - /// This is a canonical include for the symbol and can be different from - /// FileURI in the CanonicalDeclaration. - llvm::StringRef IncludeHeader; - }; - - // Optional details of the symbol. - const Details *Detail = nullptr; + /// Documentation including comment for the symbol declaration. + llvm::StringRef Documentation; + /// Type when this symbol is used in an expression. (Short display form). + /// e.g. return type of a function, or type of a variable. + llvm::StringRef ReturnType; + /// This can be either a URI of the header to be #include'd for this symbol, + /// or a literal header quoted with <> or "" that is suitable to be included + /// directly. When this is a URI, the exact #include path needs to be + /// calculated according to the URI scheme. + /// + /// This is a canonical include for the symbol and can be different from + /// FileURI in the CanonicalDeclaration. + llvm::StringRef IncludeHeader; // FIXME: add all occurrences support. // FIXME: add extra fields for index scoring signals. Index: clangd/index/Index.cpp =================================================================== --- clangd/index/Index.cpp +++ clangd/index/Index.cpp @@ -90,18 +90,9 @@ Intern(S.Signature); Intern(S.CompletionSnippetSuffix); - - if (S.Detail) { - // Copy values of StringRefs into arena. - auto *Detail = Arena.Allocate(); - *Detail = *S.Detail; - // Intern the actual strings. - Intern(Detail->Documentation); - Intern(Detail->ReturnType); - Intern(Detail->IncludeHeader); - // Replace the detail pointer with our copy. - S.Detail = Detail; - } + Intern(S.Documentation); + Intern(S.ReturnType); + Intern(S.IncludeHeader); } void SymbolSlab::Builder::insert(const Symbol &S) { Index: clangd/index/Merge.h =================================================================== --- clangd/index/Merge.h +++ clangd/index/Merge.h @@ -18,7 +18,7 @@ // Merge symbols L and R, preferring data from L in case of conflict. // The two symbols must have the same ID. // Returned symbol may contain data owned by either source. -Symbol mergeSymbol(const Symbol &L, const Symbol &R, Symbol::Details *Scratch); +Symbol mergeSymbol(const Symbol &L, const Symbol &R); // mergedIndex returns a composite index based on two provided Indexes: // - the Dynamic index covers few files, but is relatively up-to-date. Index: clangd/index/Merge.cpp =================================================================== --- clangd/index/Merge.cpp +++ clangd/index/Merge.cpp @@ -42,13 +42,12 @@ SymbolSlab Dyn = std::move(DynB).build(); DenseSet SeenDynamicSymbols; - Symbol::Details Scratch; More |= Static->fuzzyFind(Req, [&](const Symbol &S) { auto DynS = Dyn.find(S.ID); if (DynS == Dyn.end()) return Callback(S); SeenDynamicSymbols.insert(S.ID); - Callback(mergeSymbol(*DynS, S, &Scratch)); + Callback(mergeSymbol(*DynS, S)); }); for (const Symbol &S : Dyn) if (!SeenDynamicSymbols.count(S.ID)) @@ -64,14 +63,13 @@ Dynamic->lookup(Req, [&](const Symbol &S) { B.insert(S); }); auto RemainingIDs = Req.IDs; - Symbol::Details Scratch; Static->lookup(Req, [&](const Symbol &S) { const Symbol *Sym = B.find(S.ID); RemainingIDs.erase(S.ID); if (!Sym) Callback(S); else - Callback(mergeSymbol(*Sym, S, &Scratch)); + Callback(mergeSymbol(*Sym, S)); }); for (const auto &ID : RemainingIDs) if (const Symbol *Sym = B.find(ID)) @@ -93,8 +91,7 @@ }; } // namespace -Symbol -mergeSymbol(const Symbol &L, const Symbol &R, Symbol::Details *Scratch) { +Symbol mergeSymbol(const Symbol &L, const Symbol &R) { assert(L.ID == R.ID); // We prefer information from TUs that saw the definition. // Classes: this is the def itself. Functions: hopefully the header decl. @@ -114,21 +111,12 @@ S.Signature = O.Signature; if (S.CompletionSnippetSuffix == "") S.CompletionSnippetSuffix = O.CompletionSnippetSuffix; - - 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->ReturnType == "") - Scratch->ReturnType = O.Detail->ReturnType; - if (Scratch->IncludeHeader == "") - Scratch->IncludeHeader = O.Detail->IncludeHeader; - S.Detail = Scratch; - } else - S.Detail = O.Detail; - } + if (S.Documentation == "") + S.Documentation = O.Documentation; + if (S.ReturnType == "") + S.ReturnType = O.ReturnType; + if (S.IncludeHeader == "") + S.IncludeHeader = O.IncludeHeader; S.Origin |= O.Origin | SymbolOrigin::Merge; return S; Index: clangd/index/SymbolCollector.cpp =================================================================== --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -405,9 +405,7 @@ } S.Signature = Signature; S.CompletionSnippetSuffix = SnippetSuffix; - Symbol::Details Detail; - Detail.IncludeHeader = Include; - S.Detail = &Detail; + S.IncludeHeader = Include; Symbols.insert(S); return true; } @@ -486,11 +484,9 @@ } S.Signature = Signature; S.CompletionSnippetSuffix = SnippetSuffix; - Symbol::Details Detail; - Detail.Documentation = Documentation; - Detail.ReturnType = ReturnType; - Detail.IncludeHeader = Include; - S.Detail = &Detail; + S.Documentation = Documentation; + S.ReturnType = ReturnType; + S.IncludeHeader = Include; S.Origin = Opts.Origin; Symbols.insert(S); Index: clangd/index/SymbolYAML.h =================================================================== --- clangd/index/SymbolYAML.h +++ clangd/index/SymbolYAML.h @@ -30,9 +30,8 @@ SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent); // Read one symbol from a YAML-stream. -// The arena must be the Input's context! (i.e. yaml::Input Input(Text, &Arena)) -// The returned symbol is backed by both Input and Arena. -Symbol SymbolFromYAML(llvm::yaml::Input &Input, llvm::BumpPtrAllocator &Arena); +// The returned symbol is backed by Input. +Symbol SymbolFromYAML(llvm::yaml::Input &Input); // Convert a single symbol to YAML-format string. // The YAML result is safe to concatenate. Index: clangd/index/SymbolYAML.cpp =================================================================== --- clangd/index/SymbolYAML.cpp +++ clangd/index/SymbolYAML.cpp @@ -66,40 +66,9 @@ } }; -template <> struct MappingTraits { - static void mapping(IO &io, Symbol::Details &Detail) { - io.mapOptional("Documentation", Detail.Documentation); - io.mapOptional("ReturnType", Detail.ReturnType); - io.mapOptional("IncludeHeader", Detail.IncludeHeader); - } -}; - -// A YamlIO normalizer for fields of type "const T*" allocated on an arena. -// Normalizes to Optional, so traits should be provided for T. -template struct ArenaPtr { - ArenaPtr(IO &) {} - ArenaPtr(IO &, const T *D) { - if (D) - Opt = *D; - } - - const T *denormalize(IO &IO) { - assert(IO.getContext() && "Expecting an arena (as context) to allocate " - "data for read symbols."); - if (!Opt) - return nullptr; - return new (*static_cast(IO.getContext())) - T(std::move(*Opt)); // Allocate a copy of Opt on the arena. - } - - llvm::Optional Opt; -}; - template <> struct MappingTraits { static void mapping(IO &IO, Symbol &Sym) { MappingNormalization NSymbolID(IO, Sym.ID); - MappingNormalization, const Symbol::Details *> - NDetail(IO, Sym.Detail); IO.mapRequired("ID", NSymbolID->HexString); IO.mapRequired("Name", Sym.Name); IO.mapRequired("Scope", Sym.Scope); @@ -112,7 +81,9 @@ false); IO.mapOptional("Signature", Sym.Signature); IO.mapOptional("CompletionSnippetSuffix", Sym.CompletionSnippetSuffix); - IO.mapOptional("Detail", NDetail->Opt); + IO.mapOptional("Documentation", Sym.Documentation); + IO.mapOptional("ReturnType", Sym.ReturnType); + IO.mapOptional("IncludeHeader", Sym.IncludeHeader); } }; @@ -169,9 +140,7 @@ namespace clangd { SymbolSlab symbolsFromYAML(llvm::StringRef YAMLContent) { - // Store data of pointer fields (excl. `StringRef`) like `Detail`. - llvm::BumpPtrAllocator Arena; - llvm::yaml::Input Yin(YAMLContent, &Arena); + llvm::yaml::Input Yin(YAMLContent); std::vector S; Yin >> S; @@ -181,9 +150,7 @@ return std::move(Syms).build(); } -Symbol SymbolFromYAML(llvm::yaml::Input &Input, llvm::BumpPtrAllocator &Arena) { - // We could grab Arena out of Input, but it'd be a huge hazard for callers. - assert(Input.getContext() == &Arena); +Symbol SymbolFromYAML(llvm::yaml::Input &Input) { Symbol S; Input >> S; return S; Index: unittests/clangd/CodeCompleteTests.cpp =================================================================== --- unittests/clangd/CodeCompleteTests.cpp +++ unittests/clangd/CodeCompleteTests.cpp @@ -564,12 +564,10 @@ IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); - Symbol::Details Scratch; auto BarURI = URI::createFile(BarHeader).toString(); Symbol Sym = cls("ns::X"); Sym.CanonicalDeclaration.FileURI = BarURI; - Scratch.IncludeHeader = BarURI; - Sym.Detail = &Scratch; + Sym.IncludeHeader = BarURI; // Shoten include path based on search dirctory and insert. auto Results = completions(Server, R"cpp( @@ -595,16 +593,14 @@ IgnoreDiagnostics DiagConsumer; ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest()); - Symbol::Details Scratch; Symbol SymX = cls("ns::X"); Symbol SymY = cls("ns::Y"); std::string BarHeader = testPath("bar.h"); auto BarURI = URI::createFile(BarHeader).toString(); SymX.CanonicalDeclaration.FileURI = BarURI; SymY.CanonicalDeclaration.FileURI = BarURI; - Scratch.IncludeHeader = ""; - SymX.Detail = &Scratch; - SymY.Detail = &Scratch; + SymX.IncludeHeader = ""; + SymY.IncludeHeader = ""; // Shoten include path based on search dirctory and insert. auto Results = completions(Server, R"cpp( @@ -1179,11 +1175,9 @@ UnorderedElementsAre(Labeled("GFuncC(…)"), Labeled("GFuncD(int)"))); // Differences in header-to-insert suppress bundling. - Symbol::Details Detail; std::string DeclFile = URI::createFile(testPath("foo")).toString(); NoArgsGFunc.CanonicalDeclaration.FileURI = DeclFile; - Detail.IncludeHeader = ""; - NoArgsGFunc.Detail = &Detail; + NoArgsGFunc.IncludeHeader = ""; EXPECT_THAT( completions(Context + "int y = GFunc^", {NoArgsGFunc}, Opts).Completions, UnorderedElementsAre(AllOf(Named("GFuncC"), InsertInclude("")), @@ -1664,13 +1658,10 @@ } TEST(SignatureHelpTest, IndexDocumentation) { - Symbol::Details DocDetails; - DocDetails.Documentation = "Doc from the index"; - Symbol Foo0 = sym("foo", index::SymbolKind::Function, "@F@\\0#"); - Foo0.Detail = &DocDetails; + Foo0.Documentation = "Doc from the index"; Symbol Foo1 = sym("foo", index::SymbolKind::Function, "@F@\\0#I#"); - Foo1.Detail = &DocDetails; + Foo1.Documentation = "Doc from the index"; Symbol Foo2 = sym("foo", index::SymbolKind::Function, "@F@\\0#I#I#"); StringRef Sig0 = R"cpp( Index: unittests/clangd/FileIndexTests.cpp =================================================================== --- unittests/clangd/FileIndexTests.cpp +++ unittests/clangd/FileIndexTests.cpp @@ -177,7 +177,7 @@ Req.Query = ""; bool SeenSymbol = false; M.fuzzyFind(Req, [&](const Symbol &Sym) { - EXPECT_TRUE(Sym.Detail->IncludeHeader.empty()); + EXPECT_TRUE(Sym.IncludeHeader.empty()); SeenSymbol = true; }); EXPECT_TRUE(SeenSymbol); Index: unittests/clangd/IndexTests.cpp =================================================================== --- unittests/clangd/IndexTests.cpp +++ unittests/clangd/IndexTests.cpp @@ -198,32 +198,23 @@ R.References = 2; L.Signature = "()"; // present in left only R.CompletionSnippetSuffix = "{$1:0}"; // present in right only - Symbol::Details DetL, DetR; - DetL.ReturnType = "DetL"; - DetR.ReturnType = "DetR"; - DetR.Documentation = "--doc--"; - L.Detail = &DetL; - R.Detail = &DetR; + R.Documentation = "--doc--"; L.Origin = SymbolOrigin::Dynamic; R.Origin = SymbolOrigin::Static; - Symbol::Details Scratch; - Symbol M = mergeSymbol(L, R, &Scratch); + Symbol M = mergeSymbol(L, R); EXPECT_EQ(M.Name, "Foo"); EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h"); EXPECT_EQ(M.References, 3u); EXPECT_EQ(M.Signature, "()"); EXPECT_EQ(M.CompletionSnippetSuffix, "{$1:0}"); - ASSERT_TRUE(M.Detail); - EXPECT_EQ(M.Detail->ReturnType, "DetL"); - EXPECT_EQ(M.Detail->Documentation, "--doc--"); + EXPECT_EQ(M.Documentation, "--doc--"); EXPECT_EQ(M.Origin, SymbolOrigin::Dynamic | SymbolOrigin::Static | SymbolOrigin::Merge); } TEST(MergeTest, PreferSymbolWithDefn) { Symbol L, R; - Symbol::Details Scratch; L.ID = R.ID = SymbolID("hello"); L.CanonicalDeclaration.FileURI = "file:/left.h"; @@ -231,13 +222,13 @@ L.Name = "left"; R.Name = "right"; - Symbol M = mergeSymbol(L, R, &Scratch); + Symbol M = mergeSymbol(L, R); EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/left.h"); EXPECT_EQ(M.Definition.FileURI, ""); EXPECT_EQ(M.Name, "left"); R.Definition.FileURI = "file:/right.cpp"; // Now right will be favored. - M = mergeSymbol(L, R, &Scratch); + M = mergeSymbol(L, R); EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/right.h"); EXPECT_EQ(M.Definition.FileURI, "file:/right.cpp"); EXPECT_EQ(M.Name, "right"); Index: unittests/clangd/SymbolCollectorTests.cpp =================================================================== --- unittests/clangd/SymbolCollectorTests.cpp +++ unittests/clangd/SymbolCollectorTests.cpp @@ -39,22 +39,16 @@ MATCHER_P(Labeled, Label, "") { return (arg.Name + arg.Signature).str() == Label; } -MATCHER(HasReturnType, "") { - return arg.Detail && !arg.Detail->ReturnType.empty(); -} -MATCHER_P(ReturnType, D, "") { - return arg.Detail && arg.Detail->ReturnType == D; -} -MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; } +MATCHER(HasReturnType, "") { return !arg.ReturnType.empty(); } +MATCHER_P(ReturnType, D, "") { return arg.ReturnType == D; } +MATCHER_P(Doc, D, "") { return arg.Documentation == D; } MATCHER_P(Snippet, S, "") { return (arg.Name + arg.CompletionSnippetSuffix).str() == S; } MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; } MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; } MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; } -MATCHER_P(IncludeHeader, P, "") { - return arg.Detail && arg.Detail->IncludeHeader == P; -} +MATCHER_P(IncludeHeader, P, "") { return arg.IncludeHeader == P; } MATCHER_P(DeclRange, Pos, "") { return std::tie(arg.CanonicalDeclaration.Start.Line, arg.CanonicalDeclaration.Start.Column, @@ -696,9 +690,8 @@ Line: 1 Column: 1 IsIndexedForCodeCompletion: true -Detail: - Documentation: 'Foo doc' - ReturnType: 'int' +Documentation: 'Foo doc' +ReturnType: 'int' ... )"; const std::string YAML2 = R"(