[clang-tools-extra] r333371 - [clangd] Remove accessors for top-level decls from preamble

Ilya Biryukov via cfe-commits cfe-commits at lists.llvm.org
Mon May 28 05:23:17 PDT 2018


Author: ibiryukov
Date: Mon May 28 05:23:17 2018
New Revision: 333371

URL: http://llvm.org/viewvc/llvm-project?rev=333371&view=rev
Log:
[clangd] Remove accessors for top-level decls from preamble

Summary:
They cause lots of deserialization and are not actually used.
The ParsedAST accessor that previously returned those was renamed from
getTopLevelDecls to getLocalTopLevelDecls in order to avoid
confusion.

This change should considerably improve the speed of findDefinition
and other features that try to find AST nodes, corresponding to the
locations in the source code.

Reviewers: ioeric, sammccall

Reviewed By: sammccall

Subscribers: klimek, mehdi_amini, MaskRay, jkorous, cfe-commits

Differential Revision: https://reviews.llvm.org/D47331

Modified:
    clang-tools-extra/trunk/clangd/ClangdUnit.cpp
    clang-tools-extra/trunk/clangd/ClangdUnit.h
    clang-tools-extra/trunk/clangd/XRefs.cpp
    clang-tools-extra/trunk/unittests/clangd/TestTU.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=333371&r1=333370&r2=333371&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Mon May 28 05:23:17 2018
@@ -90,22 +90,8 @@ public:
   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;
@@ -113,14 +99,6 @@ public:
     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 @@ public:
 private:
   PathRef File;
   PreambleParsedCallback ParsedCallback;
-  std::vector<Decl *> TopLevelDecls;
-  std::vector<serialization::DeclID> TopLevelDeclIDs;
   std::vector<Inclusion> Inclusions;
   SourceManager *SourceMgr = nullptr;
 };
@@ -204,28 +180,6 @@ ParsedAST::Build(std::unique_ptr<clang::
                    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;
@@ -252,9 +206,8 @@ const Preprocessor &ParsedAST::getPrepro
   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; }
@@ -264,7 +217,7 @@ std::size_t ParsedAST::getUsedBytes() co
   // 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 {
@@ -272,21 +225,19 @@ const std::vector<Inclusion> &ParsedAST:
 }
 
 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);
@@ -431,9 +382,8 @@ CppFile::rebuildPreamble(CompilerInvocat
         " 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;

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.h?rev=333371&r1=333370&r2=333371&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h Mon May 28 05:23:17 2018
@@ -44,12 +44,10 @@ namespace clangd {
 
 // 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,6 +78,9 @@ public:
 
   ~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;
 
@@ -87,10 +88,9 @@ public:
   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 @@ private:
   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 @@ private:
 
   // 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;
 };
 

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=333371&r1=333370&r2=333371&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Mon May 28 05:23:17 2018
@@ -168,7 +168,7 @@ IdentifiedSymbol getSymbolAtPosition(Par
   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 @@ std::vector<DocumentHighlight> findDocum
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::All;
   IndexOpts.IndexFunctionLocals = true;
-  indexTopLevelDecls(AST.getASTContext(), AST.getTopLevelDecls(),
+  indexTopLevelDecls(AST.getASTContext(), AST.getLocalTopLevelDecls(),
                      DocHighlightsFinder, IndexOpts);
 
   return DocHighlightsFinder.takeHighlights();

Modified: clang-tools-extra/trunk/unittests/clangd/TestTU.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/TestTU.cpp?rev=333371&r1=333370&r2=333371&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/TestTU.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/TestTU.cpp Mon May 28 05:23:17 2018
@@ -1,5 +1,4 @@
-//===--- TestTU.cpp - Scratch source files for testing ------------*-
-//C++-*-===//
+//===--- TestTU.cpp - Scratch source files for testing --------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -73,22 +72,24 @@ const Symbol &findSymbol(const SymbolSla
 }
 
 const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
-  const NamedDecl *Result = nullptr;
-  for (const Decl *D : AST.getTopLevelDecls()) {
-    auto *ND = dyn_cast<NamedDecl>(D);
-    if (!ND || ND->getNameAsString() != QName)
-      continue;
-    if (Result) {
-      ADD_FAILURE() << "Multiple Decls named " << QName;
-      assert(false && "QName is not unique");
-    }
-    Result = ND;
-  }
-  if (!Result) {
-    ADD_FAILURE() << "No Decl named " << QName << " in AST";
-    assert(false && "No Decl with QName");
+  llvm::SmallVector<llvm::StringRef, 4> Components;
+  QName.split(Components, "::");
+
+  auto &Ctx = AST.getASTContext();
+  auto LookupDecl = [&Ctx](const DeclContext &Scope,
+                           llvm::StringRef Name) -> const NamedDecl & {
+    auto LookupRes = Scope.lookup(DeclarationName(&Ctx.Idents.get(Name)));
+    assert(!LookupRes.empty() && "Lookup failed");
+    assert(LookupRes.size() == 1 && "Lookup returned multiple results");
+    return *LookupRes.front();
+  };
+
+  const DeclContext *Scope = Ctx.getTranslationUnitDecl();
+  for (auto NameIt = Components.begin(), End = Components.end() - 1;
+       NameIt != End; ++NameIt) {
+    Scope = &cast<DeclContext>(LookupDecl(*Scope, *NameIt));
   }
-  return *Result;
+  return LookupDecl(*Scope, Components.back());
 }
 
 } // namespace clangd




More information about the cfe-commits mailing list