Author: ibiryukov Date: Fri Nov 24 05:04:21 2017 New Revision: 318944 URL: http://llvm.org/viewvc/llvm-project?rev=318944&view=rev Log: [clangd] Ensure preamble outlives the AST
Summary: In-memory preambles will not be copied anymore, so we need to make sure they outlive the AST. Reviewers: bkramer, sammccall, klimek Reviewed By: sammccall Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D40301 Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp clang-tools-extra/trunk/clangd/ClangdUnit.h Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=318944&r1=318943&r2=318944&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Fri Nov 24 05:04:21 2017 @@ -879,8 +879,7 @@ void clangd::dumpAST(ParsedAST &AST, llv llvm::Optional<ParsedAST> ParsedAST::Build(std::unique_ptr<clang::CompilerInvocation> CI, - const PrecompiledPreamble *Preamble, - ArrayRef<serialization::DeclID> PreambleDeclIDs, + std::shared_ptr<const PreambleData> Preamble, std::unique_ptr<llvm::MemoryBuffer> Buffer, std::shared_ptr<PCHContainerOperations> PCHs, IntrusiveRefCntPtr<vfs::FileSystem> VFS, @@ -889,8 +888,10 @@ ParsedAST::Build(std::unique_ptr<clang:: std::vector<DiagWithFixIts> ASTDiags; StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags); + const PrecompiledPreamble *PreamblePCH = + Preamble ? &Preamble->Preamble : nullptr; auto Clang = prepareCompilerInstance( - std::move(CI), Preamble, std::move(Buffer), std::move(PCHs), + std::move(CI), PreamblePCH, std::move(Buffer), std::move(PCHs), std::move(VFS), /*ref*/ UnitDiagsConsumer); // Recover resources if we crash before exiting this method. @@ -912,15 +913,8 @@ ParsedAST::Build(std::unique_ptr<clang:: Clang->getDiagnostics().setClient(new EmptyDiagsConsumer); std::vector<const Decl *> ParsedDecls = Action->takeTopLevelDecls(); - std::vector<serialization::DeclID> PendingDecls; - if (Preamble) { - PendingDecls.reserve(PreambleDeclIDs.size()); - PendingDecls.insert(PendingDecls.begin(), PreambleDeclIDs.begin(), - PreambleDeclIDs.end()); - } - - return ParsedAST(std::move(Clang), std::move(Action), std::move(ParsedDecls), - std::move(PendingDecls), std::move(ASTDiags)); + return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action), + std::move(ParsedDecls), std::move(ASTDiags)); } namespace { @@ -1061,24 +1055,25 @@ std::vector<Location> clangd::findDefini } void ParsedAST::ensurePreambleDeclsDeserialized() { - if (PendingTopLevelDecls.empty()) + if (PreambleDeclsDeserialized || !Preamble) return; std::vector<const Decl *> Resolved; - Resolved.reserve(PendingTopLevelDecls.size()); + Resolved.reserve(Preamble->TopLevelDeclIDs.size()); ExternalASTSource &Source = *getASTContext().getExternalSource(); - for (serialization::DeclID TopLevelDecl : PendingTopLevelDecls) { + for (serialization::DeclID TopLevelDecl : Preamble->TopLevelDeclIDs) { // Resolve the declaration ID to an actual declaration, possibly // deserializing the declaration in the process. if (Decl *D = Source.GetExternalDecl(TopLevelDecl)) Resolved.push_back(D); } - TopLevelDecls.reserve(TopLevelDecls.size() + PendingTopLevelDecls.size()); + TopLevelDecls.reserve(TopLevelDecls.size() + + Preamble->TopLevelDeclIDs.size()); TopLevelDecls.insert(TopLevelDecls.begin(), Resolved.begin(), Resolved.end()); - PendingTopLevelDecls.clear(); + PreambleDeclsDeserialized = true; } ParsedAST::ParsedAST(ParsedAST &&Other) = default; @@ -1112,14 +1107,21 @@ const std::vector<DiagWithFixIts> &Parse return Diags; } -ParsedAST::ParsedAST(std::unique_ptr<CompilerInstance> Clang, +PreambleData::PreambleData(PrecompiledPreamble Preamble, + std::vector<serialization::DeclID> TopLevelDeclIDs, + std::vector<DiagWithFixIts> Diags) + : Preamble(std::move(Preamble)), + TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)) {} + +ParsedAST::ParsedAST(std::shared_ptr<const PreambleData> Preamble, + std::unique_ptr<CompilerInstance> Clang, std::unique_ptr<FrontendAction> Action, std::vector<const Decl *> TopLevelDecls, - std::vector<serialization::DeclID> PendingTopLevelDecls, std::vector<DiagWithFixIts> Diags) - : Clang(std::move(Clang)), Action(std::move(Action)), - Diags(std::move(Diags)), TopLevelDecls(std::move(TopLevelDecls)), - PendingTopLevelDecls(std::move(PendingTopLevelDecls)) { + : Preamble(std::move(Preamble)), Clang(std::move(Clang)), + Action(std::move(Action)), Diags(std::move(Diags)), + TopLevelDecls(std::move(TopLevelDecls)), + PreambleDeclsDeserialized(false) { assert(this->Clang); assert(this->Action); } @@ -1130,12 +1132,6 @@ ParsedASTWrapper::ParsedASTWrapper(Parse ParsedASTWrapper::ParsedASTWrapper(llvm::Optional<ParsedAST> AST) : AST(std::move(AST)) {} -PreambleData::PreambleData(PrecompiledPreamble Preamble, - std::vector<serialization::DeclID> TopLevelDeclIDs, - std::vector<DiagWithFixIts> Diags) - : Preamble(std::move(Preamble)), - TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)) {} - std::shared_ptr<CppFile> CppFile::Create(PathRef FileName, tooling::CompileCommand Command, bool StorePreamblesInMemory, @@ -1330,12 +1326,8 @@ CppFile::deferRebuild(StringRef NewConte } // unlock Mutex // Prepare the Preamble and supplementary data for rebuilding AST. - const PrecompiledPreamble *PreambleForAST = nullptr; - ArrayRef<serialization::DeclID> SerializedPreambleDecls = llvm::None; std::vector<DiagWithFixIts> Diagnostics; if (NewPreamble) { - PreambleForAST = &NewPreamble->Preamble; - SerializedPreambleDecls = NewPreamble->TopLevelDeclIDs; Diagnostics.insert(Diagnostics.begin(), NewPreamble->Diags.begin(), NewPreamble->Diags.end()); } @@ -1345,9 +1337,9 @@ CppFile::deferRebuild(StringRef NewConte { trace::Span Tracer("Build"); SPAN_ATTACH(Tracer, "File", That->FileName); - NewAST = ParsedAST::Build( - std::move(CI), PreambleForAST, SerializedPreambleDecls, - std::move(ContentsBuffer), PCHs, VFS, That->Logger); + NewAST = + ParsedAST::Build(std::move(CI), std::move(NewPreamble), + std::move(ContentsBuffer), PCHs, VFS, That->Logger); } if (NewAST) { Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=318944&r1=318943&r2=318944&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/ClangdUnit.h (original) +++ clang-tools-extra/trunk/clangd/ClangdUnit.h Fri Nov 24 05:04:21 2017 @@ -48,6 +48,17 @@ struct DiagWithFixIts { llvm::SmallVector<tooling::Replacement, 1> FixIts; }; +// Stores Preamble and associated data. +struct PreambleData { + PreambleData(PrecompiledPreamble Preamble, + std::vector<serialization::DeclID> TopLevelDeclIDs, + std::vector<DiagWithFixIts> Diags); + + PrecompiledPreamble Preamble; + std::vector<serialization::DeclID> TopLevelDeclIDs; + std::vector<DiagWithFixIts> Diags; +}; + /// Stores and provides access to parsed AST. class ParsedAST { public: @@ -55,8 +66,7 @@ public: /// it is reused during parsing. static llvm::Optional<ParsedAST> Build(std::unique_ptr<clang::CompilerInvocation> CI, - const PrecompiledPreamble *Preamble, - ArrayRef<serialization::DeclID> PreambleDeclIDs, + std::shared_ptr<const PreambleData> Preamble, std::unique_ptr<llvm::MemoryBuffer> Buffer, std::shared_ptr<PCHContainerOperations> PCHs, IntrusiveRefCntPtr<vfs::FileSystem> VFS, clangd::Logger &Logger); @@ -80,15 +90,18 @@ public: const std::vector<DiagWithFixIts> &getDiagnostics() const; private: - ParsedAST(std::unique_ptr<CompilerInstance> Clang, + ParsedAST(std::shared_ptr<const PreambleData> Preamble, + std::unique_ptr<CompilerInstance> Clang, std::unique_ptr<FrontendAction> Action, std::vector<const Decl *> TopLevelDecls, - std::vector<serialization::DeclID> PendingTopLevelDecls, std::vector<DiagWithFixIts> Diags); private: void ensurePreambleDeclsDeserialized(); + // In-memory preambles must outlive the AST, it is important that this member + // goes before Clang and Action. + std::shared_ptr<const PreambleData> Preamble; // We store an "incomplete" FrontendAction (i.e. no EndSourceFile was called // on it) and CompilerInstance used to run it. That way we don't have to do // complex memory management of all Clang structures on our own. (They are @@ -100,7 +113,7 @@ private: // Data, stored after parsing. std::vector<DiagWithFixIts> Diags; std::vector<const Decl *> TopLevelDecls; - std::vector<serialization::DeclID> PendingTopLevelDecls; + bool PreambleDeclsDeserialized; }; // Provides thread-safe access to ParsedAST. @@ -124,17 +137,6 @@ private: mutable llvm::Optional<ParsedAST> AST; }; -// Stores Preamble and associated data. -struct PreambleData { - PreambleData(PrecompiledPreamble Preamble, - std::vector<serialization::DeclID> TopLevelDeclIDs, - std::vector<DiagWithFixIts> Diags); - - PrecompiledPreamble Preamble; - std::vector<serialization::DeclID> TopLevelDeclIDs; - std::vector<DiagWithFixIts> Diags; -}; - /// Manages resources, required by clangd. Allows to rebuild file with new /// contents, and provides AST and Preamble for it. class CppFile : public std::enable_shared_from_this<CppFile> { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits