[clang-tools-extra] r318944 - [clangd] Ensure preamble outlives the AST

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 24 05:04:21 PST 2017


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> {




More information about the cfe-commits mailing list