ilya-biryukov updated this revision to Diff 148602.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Rename the field in addition to the getter
- Address review comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D47331

Files:
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/XRefs.cpp
  unittests/clangd/TestTU.cpp

Index: unittests/clangd/TestTU.cpp
===================================================================
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -74,7 +74,7 @@
 
 const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
   const NamedDecl *Result = nullptr;
-  for (const Decl *D : AST.getTopLevelDecls()) {
+  for (const Decl *D : AST.getLocalTopLevelDecls()) {
     auto *ND = dyn_cast<NamedDecl>(D);
     if (!ND || ND->getNameAsString() != QName)
       continue;
Index: clangd/XRefs.cpp
===================================================================
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -168,7 +168,7 @@
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
-  indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+  indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
                      DeclMacrosFinder, IndexOpts);
 
   return {DeclMacrosFinder.takeDecls(), DeclMacrosFinder.takeMacroInfos()};
@@ -418,7 +418,7 @@
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
-  indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+  indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
                      DocHighlightsFinder, IndexOpts);
 
   return DocHighlightsFinder.takeHighlights();
Index: clangd/ClangdUnit.h
===================================================================
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -44,12 +44,10 @@
 
 // Stores Preamble and associated data.
 struct PreambleData {
-  PreambleData(PrecompiledPreamble Preamble,
-               std::vector<serialization::DeclID> TopLevelDeclIDs,
-               std::vector<Diag> Diags, std::vector<Inclusion> Inclusions);
+  PreambleData(PrecompiledPreamble Preamble, std::vector<Diag> Diags,
+               std::vector<Inclusion> Inclusions);
 
   PrecompiledPreamble Preamble;
-  std::vector<serialization::DeclID> TopLevelDeclIDs;
   std::vector<Diag> Diags;
   // Processes like code completions and go-to-definitions will need #include
   // information, and their compile action skips preamble range.
@@ -80,17 +78,19 @@
 
   ~ParsedAST();
 
+  /// Note that the returned ast will not contain decls from the preamble that
+  /// were not deserialized during parsing. Clients should expect only decls
+  /// from the main file to be in the AST.
   ASTContext &getASTContext();
   const ASTContext &getASTContext() const;
 
   Preprocessor &getPreprocessor();
   std::shared_ptr<Preprocessor> getPreprocessorPtr();
   const Preprocessor &getPreprocessor() const;
 
-  /// This function returns all top-level decls, including those that come
-  /// from Preamble. Decls, coming from Preamble, have to be deserialized, so
-  /// this call might be expensive.
-  ArrayRef<const Decl *> getTopLevelDecls();
+  /// This function returns top-level decls present in the main file of the AST.
+  /// The result does not include the decls that come from the preamble.
+  ArrayRef<const Decl *> getLocalTopLevelDecls();
 
   const std::vector<Diag> &getDiagnostics() const;
 
@@ -103,11 +103,8 @@
   ParsedAST(std::shared_ptr<const PreambleData> Preamble,
             std::unique_ptr<CompilerInstance> Clang,
             std::unique_ptr<FrontendAction> Action,
-            std::vector<const Decl *> TopLevelDecls, std::vector<Diag> Diags,
-            std::vector<Inclusion> Inclusions);
-
-private:
-  void ensurePreambleDeclsDeserialized();
+            std::vector<const Decl *> LocalTopLevelDecls,
+            std::vector<Diag> Diags, std::vector<Inclusion> Inclusions);
 
   // In-memory preambles must outlive the AST, it is important that this member
   // goes before Clang and Action.
@@ -122,8 +119,9 @@
 
   // Data, stored after parsing.
   std::vector<Diag> Diags;
-  std::vector<const Decl *> TopLevelDecls;
-  bool PreambleDeclsDeserialized;
+  // Top-level decls inside the current file. Not that this does not include
+  // top-level decls from the preamble.
+  std::vector<const Decl *> LocalTopLevelDecls;
   std::vector<Inclusion> Inclusions;
 };
 
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -90,37 +90,15 @@
   CppFilePreambleCallbacks(PathRef File, PreambleParsedCallback ParsedCallback)
       : File(File), ParsedCallback(ParsedCallback) {}
 
-  std::vector<serialization::DeclID> takeTopLevelDeclIDs() {
-    return std::move(TopLevelDeclIDs);
-  }
-
   std::vector<Inclusion> takeInclusions() { return std::move(Inclusions); }
 
-  void AfterPCHEmitted(ASTWriter &Writer) override {
-    TopLevelDeclIDs.reserve(TopLevelDecls.size());
-    for (Decl *D : TopLevelDecls) {
-      // Invalid top-level decls may not have been serialized.
-      if (D->isInvalidDecl())
-        continue;
-      TopLevelDeclIDs.push_back(Writer.getDeclID(D));
-    }
-  }
-
   void AfterExecute(CompilerInstance &CI) override {
     if (!ParsedCallback)
       return;
     trace::Span Tracer("Running PreambleCallback");
     ParsedCallback(File, CI.getASTContext(), CI.getPreprocessorPtr());
   }
 
-  void HandleTopLevelDecl(DeclGroupRef DG) override {
-    for (Decl *D : DG) {
-      if (isa<ObjCMethodDecl>(D))
-        continue;
-      TopLevelDecls.push_back(D);
-    }
-  }
-
   void BeforeExecute(CompilerInstance &CI) override {
     SourceMgr = &CI.getSourceManager();
   }
@@ -135,8 +113,6 @@
 private:
   PathRef File;
   PreambleParsedCallback ParsedCallback;
-  std::vector<Decl *> TopLevelDecls;
-  std::vector<serialization::DeclID> TopLevelDeclIDs;
   std::vector<Inclusion> Inclusions;
   SourceManager *SourceMgr = nullptr;
 };
@@ -200,28 +176,6 @@
                    std::move(Inclusions));
 }
 
-void ParsedAST::ensurePreambleDeclsDeserialized() {
-  if (PreambleDeclsDeserialized || !Preamble)
-    return;
-
-  std::vector<const Decl *> Resolved;
-  Resolved.reserve(Preamble->TopLevelDeclIDs.size());
-
-  ExternalASTSource &Source = *getASTContext().getExternalSource();
-  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() +
-                        Preamble->TopLevelDeclIDs.size());
-  TopLevelDecls.insert(TopLevelDecls.begin(), Resolved.begin(), Resolved.end());
-
-  PreambleDeclsDeserialized = true;
-}
-
 ParsedAST::ParsedAST(ParsedAST &&Other) = default;
 
 ParsedAST &ParsedAST::operator=(ParsedAST &&Other) = default;
@@ -248,9 +202,8 @@
   return Clang->getPreprocessor();
 }
 
-ArrayRef<const Decl *> ParsedAST::getTopLevelDecls() {
-  ensurePreambleDeclsDeserialized();
-  return TopLevelDecls;
+ArrayRef<const Decl *> ParsedAST::getLocalTopLevelDecls() {
+  return LocalTopLevelDecls;
 }
 
 const std::vector<Diag> &ParsedAST::getDiagnostics() const { return Diags; }
@@ -260,29 +213,27 @@
   // FIXME(ibiryukov): we do not account for the dynamically allocated part of
   // Message and Fixes inside each diagnostic.
   return AST.getASTAllocatedMemory() + AST.getSideTableAllocatedMemory() +
-         ::getUsedBytes(TopLevelDecls) + ::getUsedBytes(Diags);
+         ::getUsedBytes(LocalTopLevelDecls) + ::getUsedBytes(Diags);
 }
 
 const std::vector<Inclusion> &ParsedAST::getInclusions() const {
   return Inclusions;
 }
 
 PreambleData::PreambleData(PrecompiledPreamble Preamble,
-                           std::vector<serialization::DeclID> TopLevelDeclIDs,
                            std::vector<Diag> Diags,
                            std::vector<Inclusion> Inclusions)
-    : Preamble(std::move(Preamble)),
-      TopLevelDeclIDs(std::move(TopLevelDeclIDs)), Diags(std::move(Diags)),
+    : Preamble(std::move(Preamble)), Diags(std::move(Diags)),
       Inclusions(std::move(Inclusions)) {}
 
 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<const Decl *> LocalTopLevelDecls,
                      std::vector<Diag> Diags, std::vector<Inclusion> Inclusions)
     : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
       Action(std::move(Action)), Diags(std::move(Diags)),
-      TopLevelDecls(std::move(TopLevelDecls)), PreambleDeclsDeserialized(false),
+      LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
       Inclusions(std::move(Inclusions)) {
   assert(this->Clang);
   assert(this->Action);
@@ -427,9 +378,8 @@
         " for file " + Twine(FileName));
 
     return std::make_shared<PreambleData>(
-        std::move(*BuiltPreamble),
-        SerializedDeclsCollector.takeTopLevelDeclIDs(),
-        PreambleDiagnostics.take(), SerializedDeclsCollector.takeInclusions());
+        std::move(*BuiltPreamble), PreambleDiagnostics.take(),
+        SerializedDeclsCollector.takeInclusions());
   } else {
     log("Could not build a preamble for file " + Twine(FileName));
     return nullptr;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to