[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D35406#821567, @petarj wrote:

> Can you check if this change introduced clang-x86-windows-msvc2015 buildbot 
> failure 
> ?


It did. Was fixed in r308959 .


Repository:
  rL LLVM

https://reviews.llvm.org/D35406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Petar Jovanovic via Phabricator via cfe-commits]

petarj added a comment.

Can you check if this change introduced clang-x86-windows-msvc2015 buildbot 
failure 
?


Repository:
  rL LLVM

https://reviews.llvm.org/D35406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL308738: [clangd] Replace ASTUnit with manual AST management. 
(authored by ibiryukov).

Changed prior to commit:
  https://reviews.llvm.org/D35406?vs=107475=107666#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35406

Files:
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h

Index: clang-tools-extra/trunk/clangd/ClangdUnit.h
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.h
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h
@@ -13,6 +13,9 @@
 #include "Path.h"
 #include "Protocol.h"
 #include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/PrecompiledPreamble.h"
+#include "clang/Serialization/ASTBitCodes.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include 
 
@@ -70,11 +73,82 @@
   void dumpAST(llvm::raw_ostream ) const;
 
 private:
+  /// Stores and provides access to parsed AST.
+  class ParsedAST {
+  public:
+/// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
+/// it is reused during parsing.
+static llvm::Optional
+Build(std::unique_ptr CI,
+  const PrecompiledPreamble *Preamble,
+  ArrayRef PreambleDeclIDs,
+  std::unique_ptr Buffer,
+  std::shared_ptr PCHs,
+  IntrusiveRefCntPtr VFS);
+
+ParsedAST(ParsedAST &);
+ParsedAST =(ParsedAST &);
+
+~ParsedAST();
+
+ASTContext ();
+const ASTContext () const;
+
+Preprocessor ();
+const Preprocessor () 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 getTopLevelDecls();
+
+const std::vector () const;
+
+  private:
+ParsedAST(std::unique_ptr Clang,
+  std::unique_ptr Action,
+  std::vector TopLevelDecls,
+  std::vector PendingTopLevelDecls,
+  std::vector Diags);
+
+  private:
+void ensurePreambleDeclsDeserialized();
+
+// 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
+// stored in CompilerInstance and cleaned up by
+// FrontendAction.EndSourceFile).
+std::unique_ptr Clang;
+std::unique_ptr Action;
+
+// Data, stored after parsing.
+std::vector Diags;
+std::vector TopLevelDecls;
+std::vector PendingTopLevelDecls;
+  };
+
+  // Store Preamble and all associated data
+  struct PreambleData {
+PreambleData(PrecompiledPreamble Preamble,
+ std::vector TopLevelDeclIDs,
+ std::vector Diags);
+
+PrecompiledPreamble Preamble;
+std::vector TopLevelDeclIDs;
+std::vector Diags;
+  };
+
+  SourceLocation getBeginningOfIdentifier(const Position ,
+  const FileEntry *FE) const;
+
   Path FileName;
-  std::unique_ptr Unit;
-  std::shared_ptr PCHs;
+  tooling::CompileCommand Command;
 
-  SourceLocation getBeginningOfIdentifier(const Position& Pos, const FileEntry* FE) const;
+  llvm::Optional Preamble;
+  llvm::Optional Unit;
+
+  std::shared_ptr PCHs;
 };
 
 } // namespace clangd
Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -9,23 +9,219 @@
 
 #include "ClangdUnit.h"
 
-#include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/Utils.h"
-#include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/Format.h"
 
 #include 
 
 using namespace clang::clangd;
 using namespace clang;
 
+namespace {
+
+class DeclTrackingASTConsumer : public ASTConsumer {
+public:
+  DeclTrackingASTConsumer(std::vector )
+  : TopLevelDecls(TopLevelDecls) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (const Decl *D : DG) {
+  // ObjCMethodDecl are not actually top-level decls.
+  if (isa(D))
+continue;
+
+  

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir accepted this revision.
krasimir added a comment.
This revision is now accepted and ready to land.

Looks good!


https://reviews.llvm.org/D35406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 107475.
ilya-biryukov added a comment.

Added a few comments.


https://reviews.llvm.org/D35406

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h

Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -13,6 +13,9 @@
 #include "Path.h"
 #include "Protocol.h"
 #include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/PrecompiledPreamble.h"
+#include "clang/Serialization/ASTBitCodes.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include 
 
@@ -70,11 +73,82 @@
   void dumpAST(llvm::raw_ostream ) const;
 
 private:
+  /// Stores and provides access to parsed AST.
+  class ParsedAST {
+  public:
+/// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
+/// it is reused during parsing.
+static llvm::Optional
+Build(std::unique_ptr CI,
+  const PrecompiledPreamble *Preamble,
+  ArrayRef PreambleDeclIDs,
+  std::unique_ptr Buffer,
+  std::shared_ptr PCHs,
+  IntrusiveRefCntPtr VFS);
+
+ParsedAST(ParsedAST &);
+ParsedAST =(ParsedAST &);
+
+~ParsedAST();
+
+ASTContext ();
+const ASTContext () const;
+
+Preprocessor ();
+const Preprocessor () 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 getTopLevelDecls();
+
+const std::vector () const;
+
+  private:
+ParsedAST(std::unique_ptr Clang,
+  std::unique_ptr Action,
+  std::vector TopLevelDecls,
+  std::vector PendingTopLevelDecls,
+  std::vector Diags);
+
+  private:
+void ensurePreambleDeclsDeserialized();
+
+// 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
+// stored in CompilerInstance and cleaned up by
+// FrontendAction.EndSourceFile).
+std::unique_ptr Clang;
+std::unique_ptr Action;
+
+// Data, stored after parsing.
+std::vector Diags;
+std::vector TopLevelDecls;
+std::vector PendingTopLevelDecls;
+  };
+
+  // Store Preamble and all associated data
+  struct PreambleData {
+PreambleData(PrecompiledPreamble Preamble,
+ std::vector TopLevelDeclIDs,
+ std::vector Diags);
+
+PrecompiledPreamble Preamble;
+std::vector TopLevelDeclIDs;
+std::vector Diags;
+  };
+
+  SourceLocation getBeginningOfIdentifier(const Position ,
+  const FileEntry *FE) const;
+
   Path FileName;
-  std::unique_ptr Unit;
-  std::shared_ptr PCHs;
+  tooling::CompileCommand Command;
 
-  SourceLocation getBeginningOfIdentifier(const Position& Pos, const FileEntry* FE) const;
+  llvm::Optional Preamble;
+  llvm::Optional Unit;
+
+  std::shared_ptr PCHs;
 };
 
 } // namespace clangd
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -9,23 +9,219 @@
 
 #include "ClangdUnit.h"
 
-#include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/Utils.h"
-#include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/Format.h"
 
 #include 
 
 using namespace clang::clangd;
 using namespace clang;
 
+namespace {
+
+class DeclTrackingASTConsumer : public ASTConsumer {
+public:
+  DeclTrackingASTConsumer(std::vector )
+  : TopLevelDecls(TopLevelDecls) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (const Decl *D : DG) {
+  // ObjCMethodDecl are not actually top-level decls.
+  if (isa(D))
+continue;
+
+  TopLevelDecls.push_back(D);
+}
+return true;
+  }
+
+private:
+  std::vector 
+};
+
+class ClangdFrontendAction : public SyntaxOnlyAction {
+public:
+  std::vector takeTopLevelDecls() {
+return std::move(TopLevelDecls);
+  }
+
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef InFile) override {
+return 

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdUnit.cpp:167
+std::move(VFS));
+  CI->getFrontendOpts().DisableFree = false;
+  return CI;

krasimir wrote:
> Why `DisableFree`?
We rely on CompilerInstance to free the resources. It won't do that if 
`DisableFree` is set to true. Added a comment about that.



Comment at: clangd/ClangdUnit.cpp:251
+CompilerInstance::createDiagnostics(new DiagnosticOptions,
+, false);
+CI = createCompilerInvocation(ArgStrs, CommandLineDiagsEngine, VFS);

krasimir wrote:
> What's the purpose of this local scope here?
> Wouldn't  point to garbage after the end of this 
> local scope?
This `DiagnosticsEngine` is not stored in `CompilerInvocation` created here, it 
is used for reporting errors during command-line parsing.
Since we don't show those errors to the user, we discard them.

Local scope is there to avoid referencing `CommandLineDiagsEngine` later (as it 
will discard diagnostics, and we actually want to have them).



Comment at: clangd/ClangdUnit.cpp:262
+  ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
+  if (!Preamble || !Preamble->Preamble.CanReuse(*CI, ContentsBuffer.get(),
+Bounds, VFS.get())) {

krasimir wrote:
> We can not compute `Bounds`, `if (Preamble)`.
When `Preamble` is set, we need `Bounds` for `Preamble.CanReuse`.
When `Preamble` is not set, we need `Bounds` for `PrecompiledPreamble::Build`.

So we need them in both cases.



Comment at: clangd/ClangdUnit.cpp:268
+CompilerInstance::createDiagnostics(
+>getDiagnosticOpts(), , false);
+ClangdUnitPreambleCallbacks SerializedDeclsCollector;

krasimir wrote:
> Here `PreambleDiagsEngine` holds a pointer to the local variable 
> `PreambleDiagnosticsConsumer`.
> Next, `BuiltPreamble` holds a reference to `PreambleDiagsEngine`.
> Next, `Preamble` is created by moving `BuiltPreamble`.
> Doesn't then `Preamble` hold a transitive reference to junk after this local 
> scope?
I don't think `BuiltPreamble` holds a reference to `PreambleDiagsEngine`.  Here 
is a list of fields of `PrecompiledPreamble` class:


```
  //...
  TempPCHFile PCHFile;
  llvm::StringMap FilesInPreamble;
  std::vector PreambleBytes;
  bool PreambleEndsAtStartOfLine;
```




Comment at: clangd/ClangdUnit.cpp:404
+CI = createCompilerInvocation(ArgStrs, CommandLineDiagsEngine, VFS);
+  }
+  assert(CI && "Couldn't create CompilerInvocation");

krasimir wrote:
> what's the purpose of this local scope?
The `DiagnosticsEngine` used here uses default `DiagnosticOptions`.
A one that's properly configured after reading command-line arguments is 
created below, inside a `prepareCompilerInstance` call.
To avoid using `DiagnosticsEngine` configured by default, it is put into a 
local scope.



Comment at: clangd/ClangdUnit.cpp:691
+
+void ClangdUnit::ParsedAST::ensurePreambleDeclsDeserialized() {
+  if (PendingTopLevelDecls.empty())

krasimir wrote:
> Why do we need to ensure that Decls are deserialized?
These Decls are used in `findDefinitions` (specifically, they are passed to 
`indexTopLevelDecls`). I've opted for mimicing the `ASTUnit` behaviour here.

It is possible that visiting only local decls (i.e. not from `Preamble`) will 
not break anything. But that requires thorough testing and digging into how 
PCHs works (one question is, if it is possible to have decls from the same file 
as part of the PCH). I actually want to investigate if we actually need all 
that deserializing logic here, but would prefer to do that with a separate 
change to keep semantic changes related to this commit as small as possible.

PS. Alternatively, we could just visit all decls in `ASTContext` whenever we 
need them for `findDefinitions`, but [[ 
https://reviews.llvm.org/diffusion/L/browse/cfe/trunk/include/clang/Frontend/ASTUnit.h;308597$149
 | a comment in ASTUnit]] says it's slower.



Comment at: clangd/ClangdUnit.h:77
+  /// Stores and provides access to parsed AST.
+  class ParsedAST {
+  public:

krasimir wrote:
> Why is this a separate class and not just part of `ClangdUnit`?
For managing the lifetime of an AST. Specifically:
 - there is a destructor here that calls `EndSourceFile` to free AST-related 
resources,
 - all getters return references to objects that either have pointers to 
resources, managed by `ParsedAST` (`getASTContext`, `getPreprocessor`, etc) or 
semantically tied to this specific parsing result(`getDiagnostics`, 
`PendingTopLevelDecls`).

`ClangdUnit` is managing two things at the moment: a preamble and an AST. Since 
the lifetime of those two is different, it is arguably a good idea to separate 
them.

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: clangd/ClangdUnit.cpp:167
+std::move(VFS));
+  CI->getFrontendOpts().DisableFree = false;
+  return CI;

Why `DisableFree`?



Comment at: clangd/ClangdUnit.cpp:251
+CompilerInstance::createDiagnostics(new DiagnosticOptions,
+, false);
+CI = createCompilerInvocation(ArgStrs, CommandLineDiagsEngine, VFS);

What's the purpose of this local scope here?
Wouldn't  point to garbage after the end of this local 
scope?



Comment at: clangd/ClangdUnit.cpp:262
+  ComputePreambleBounds(*CI->getLangOpts(), ContentsBuffer.get(), 0);
+  if (!Preamble || !Preamble->Preamble.CanReuse(*CI, ContentsBuffer.get(),
+Bounds, VFS.get())) {

We can not compute `Bounds`, `if (Preamble)`.



Comment at: clangd/ClangdUnit.cpp:268
+CompilerInstance::createDiagnostics(
+>getDiagnosticOpts(), , false);
+ClangdUnitPreambleCallbacks SerializedDeclsCollector;

Here `PreambleDiagsEngine` holds a pointer to the local variable 
`PreambleDiagnosticsConsumer`.
Next, `BuiltPreamble` holds a reference to `PreambleDiagsEngine`.
Next, `Preamble` is created by moving `BuiltPreamble`.
Doesn't then `Preamble` hold a transitive reference to junk after this local 
scope?



Comment at: clangd/ClangdUnit.cpp:404
+CI = createCompilerInvocation(ArgStrs, CommandLineDiagsEngine, VFS);
+  }
+  assert(CI && "Couldn't create CompilerInvocation");

what's the purpose of this local scope?



Comment at: clangd/ClangdUnit.cpp:691
+
+void ClangdUnit::ParsedAST::ensurePreambleDeclsDeserialized() {
+  if (PendingTopLevelDecls.empty())

Why do we need to ensure that Decls are deserialized?



Comment at: clangd/ClangdUnit.h:77
+  /// Stores and provides access to parsed AST.
+  class ParsedAST {
+  public:

Why is this a separate class and not just part of `ClangdUnit`?


https://reviews.llvm.org/D35406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D35406#809623, @malaperle wrote:

> For synching with what I am doing. I am "only" looking right now at the 
> modeling of the index and its on-disk storage, not really on the speeding up 
> of the parsing of the TUs (i.e. the input of the index). I use 
> index::IndexDataConsumer to feed the index and I see that in your change it 
> is still pretty much used the same way as before so there is no problem. I 
> will post on the indexing thread from before with a more detailed proposal 
> from before once it is a bit further along.


Great that this change is not interfering. Eager to see your proposal for the 
indexing.


https://reviews.llvm.org/D35406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

In https://reviews.llvm.org/D35406#809609, @ilya-biryukov wrote:

> The idea is to allows us changing the way we manage/rebuild PCHs and ASTs.
>  ASTUnit is used for many things and has a fairly complicated and verbose 
> interface and does a lot of mutations all other the place inside it, so 
> making changes to it is not particularly easy.
>  On the other hand, it doesn't add a lot of value for clangd specifically, 
> since we use a very simple subset of it.
>
> The next step I planned was to allow code completion to be run in parallel 
> with recomputing preamble/computing diagnostics. (And in general, allow 
> multiple TUs to be processed in parallel).
>  That would probably involve changes to the interface, so we should 
> definitely sync with you on the work you've been doing.
>  And we were also planning to look into indexing at a later point, so 
> discussing the design there might be interesting as well.


Thanks a lot for the additional information.

For synching with what I am doing. I am "only" looking right now at the 
modeling of the index and its on-disk storage, not really on the speeding up of 
the parsing of the TUs (i.e. the input of the index). I use 
index::IndexDataConsumer to feed the index and I see that in your change it is 
still pretty much used the same way as before so there is no problem. I will 
post on the indexing thread from before with a more detailed proposal from 
before once it is a bit further along.


https://reviews.llvm.org/D35406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The idea is to allows us changing the way we manage/rebuild PCHs and ASTs.
ASTUnit is used for many things and has a fairly complicated and verbose 
interface and does a lot of mutations all other the place inside it, so making 
changes to it is not particularly easy.
On the other hand, it doesn't add a lot of value for clangd specifically, since 
we use a very simple subset of it.

The next step I planned was to allow code completion to be run in parallel with 
recomputing preamble/computing diagnostics. (And in general, allow multiple TUs 
to be processed in parallel).
That would probably involve changes to the interface, so we should definitely 
sync with you on the work you've been doing.
And we were also planning to look into indexing at a later point, so discussing 
the design there might be interesting as well.


https://reviews.llvm.org/D35406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

Could you explain what the goal of this change is? It would help understand how 
it will impact the indexing work I am currently doing.


https://reviews.llvm.org/D35406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 106632.
ilya-biryukov added a comment.

- Replaced TODO with FIXME in a comment.


https://reviews.llvm.org/D35406

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h

Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -13,6 +13,9 @@
 #include "Path.h"
 #include "Protocol.h"
 #include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/PrecompiledPreamble.h"
+#include "clang/Serialization/ASTBitCodes.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include 
 
@@ -70,11 +73,79 @@
   void dumpAST(llvm::raw_ostream ) const;
 
 private:
+  /// Stores and provides access to parsed AST.
+  class ParsedAST {
+  public:
+/// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
+/// it is reused during parsing.
+static llvm::Optional
+Build(std::unique_ptr CI,
+  const PrecompiledPreamble *Preamble,
+  ArrayRef PreambleDeclIDs,
+  std::unique_ptr Buffer,
+  std::shared_ptr PCHs,
+  IntrusiveRefCntPtr VFS);
+
+ParsedAST(ParsedAST &);
+ParsedAST =(ParsedAST &);
+
+~ParsedAST();
+
+ASTContext ();
+const ASTContext () const;
+
+Preprocessor ();
+const Preprocessor () const;
+
+ArrayRef getTopLevelDecls();
+
+const std::vector () const;
+
+  private:
+ParsedAST(std::unique_ptr Clang,
+  std::unique_ptr Action,
+  std::vector TopLevelDecls,
+  std::vector PendingTopLevelDecls,
+  std::vector Diags);
+
+  private:
+void ensurePreambleDeclsDeserialized();
+
+// 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
+// stored in CompilerInstance and cleaned up by
+// FrontendAction.EndSourceFile).
+std::unique_ptr Clang;
+std::unique_ptr Action;
+
+// Data, stored after parsing.
+std::vector Diags;
+std::vector TopLevelDecls;
+std::vector PendingTopLevelDecls;
+  };
+
+  // Store Preamble and all associated data
+  struct PreambleData {
+PreambleData(PrecompiledPreamble Preamble,
+ std::vector TopLevelDeclIDs,
+ std::vector Diags);
+
+PrecompiledPreamble Preamble;
+std::vector TopLevelDeclIDs;
+std::vector Diags;
+  };
+
+  SourceLocation getBeginningOfIdentifier(const Position ,
+  const FileEntry *FE) const;
+
   Path FileName;
-  std::unique_ptr Unit;
-  std::shared_ptr PCHs;
+  tooling::CompileCommand Command;
 
-  SourceLocation getBeginningOfIdentifier(const Position& Pos, const FileEntry* FE) const;
+  llvm::Optional Preamble;
+  llvm::Optional Unit;
+
+  std::shared_ptr PCHs;
 };
 
 } // namespace clangd
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -9,23 +9,215 @@
 
 #include "ClangdUnit.h"
 
-#include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/Utils.h"
-#include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/Format.h"
 
 #include 
 
 using namespace clang::clangd;
 using namespace clang;
 
+namespace {
+
+class DeclTrackingASTConsumer : public ASTConsumer {
+public:
+  DeclTrackingASTConsumer(std::vector )
+  : TopLevelDecls(TopLevelDecls) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (const Decl *D : DG) {
+  // ObjCMethodDecl are not actually top-level decls.
+  if (isa(D))
+continue;
+
+  TopLevelDecls.push_back(D);
+}
+return true;
+  }
+
+private:
+  std::vector 
+};
+
+class ClangdFrontendAction : public SyntaxOnlyAction {
+public:
+  std::vector takeTopLevelDecls() {
+return std::move(TopLevelDecls);
+  }
+
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef InFile) override {
+return llvm::make_unique(/*ref*/ TopLevelDecls);
+  }
+
+private:
+  std::vector TopLevelDecls;
+};
+
+class ClangdUnitPreambleCallbacks : public PreambleCallbacks {
+public:
+  std::vector 

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Depends on change to cfe (https://reviews.llvm.org/D35405) to compile properly.


https://reviews.llvm.org/D35406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35406: [clangd] Replace ASTUnit with manual AST management.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.

This refactoring does not aim to introduce any significant changes to
the behaviour of clangd to keep the change as simple as possible.


https://reviews.llvm.org/D35406

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h

Index: clangd/ClangdUnit.h
===
--- clangd/ClangdUnit.h
+++ clangd/ClangdUnit.h
@@ -13,6 +13,9 @@
 #include "Path.h"
 #include "Protocol.h"
 #include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/PrecompiledPreamble.h"
+#include "clang/Serialization/ASTBitCodes.h"
+#include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include 
 
@@ -70,11 +73,79 @@
   void dumpAST(llvm::raw_ostream ) const;
 
 private:
+  /// Stores and provides access to parsed AST.
+  class ParsedAST {
+  public:
+/// Attempts to run Clang and store parsed AST. If \p Preamble is non-null
+/// it is reused during parsing.
+static llvm::Optional
+Build(std::unique_ptr CI,
+  const PrecompiledPreamble *Preamble,
+  ArrayRef PreambleDeclIDs,
+  std::unique_ptr Buffer,
+  std::shared_ptr PCHs,
+  IntrusiveRefCntPtr VFS);
+
+ParsedAST(ParsedAST &);
+ParsedAST =(ParsedAST &);
+
+~ParsedAST();
+
+ASTContext ();
+const ASTContext () const;
+
+Preprocessor ();
+const Preprocessor () const;
+
+ArrayRef getTopLevelDecls();
+
+const std::vector () const;
+
+  private:
+ParsedAST(std::unique_ptr Clang,
+  std::unique_ptr Action,
+  std::vector TopLevelDecls,
+  std::vector PendingTopLevelDecls,
+  std::vector Diags);
+
+  private:
+void ensurePreambleDeclsDeserialized();
+
+// 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
+// stored in CompilerInstance and cleaned up by
+// FrontendAction.EndSourceFile).
+std::unique_ptr Clang;
+std::unique_ptr Action;
+
+// Data, stored after parsing.
+std::vector Diags;
+std::vector TopLevelDecls;
+std::vector PendingTopLevelDecls;
+  };
+
+  // Store Preamble and all associated data
+  struct PreambleData {
+PreambleData(PrecompiledPreamble Preamble,
+ std::vector TopLevelDeclIDs,
+ std::vector Diags);
+
+PrecompiledPreamble Preamble;
+std::vector TopLevelDeclIDs;
+std::vector Diags;
+  };
+
+  SourceLocation getBeginningOfIdentifier(const Position ,
+  const FileEntry *FE) const;
+
   Path FileName;
-  std::unique_ptr Unit;
-  std::shared_ptr PCHs;
+  tooling::CompileCommand Command;
 
-  SourceLocation getBeginningOfIdentifier(const Position& Pos, const FileEntry* FE) const;
+  llvm::Optional Preamble;
+  llvm::Optional Unit;
+
+  std::shared_ptr PCHs;
 };
 
 } // namespace clangd
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -9,23 +9,215 @@
 
 #include "ClangdUnit.h"
 
-#include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
+#include "clang/Frontend/FrontendActions.h"
 #include "clang/Frontend/Utils.h"
-#include "clang/Index/IndexingAction.h"
 #include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Serialization/ASTWriter.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/CrashRecoveryContext.h"
 #include "llvm/Support/Format.h"
 
 #include 
 
 using namespace clang::clangd;
 using namespace clang;
 
+namespace {
+
+class DeclTrackingASTConsumer : public ASTConsumer {
+public:
+  DeclTrackingASTConsumer(std::vector )
+  : TopLevelDecls(TopLevelDecls) {}
+
+  bool HandleTopLevelDecl(DeclGroupRef DG) override {
+for (const Decl *D : DG) {
+  // ObjCMethodDecl are not actually top-level decls.
+  if (isa(D))
+continue;
+
+  TopLevelDecls.push_back(D);
+}
+return true;
+  }
+
+private:
+  std::vector 
+};
+
+class ClangdFrontendAction : public SyntaxOnlyAction {
+public:
+  std::vector takeTopLevelDecls() {
+return std::move(TopLevelDecls);
+  }
+
+protected:
+  std::unique_ptr CreateASTConsumer(CompilerInstance ,
+ StringRef InFile) override {
+return llvm::make_unique(/*ref*/ TopLevelDecls);
+  }
+
+private:
+  std::vector TopLevelDecls;
+};
+
+class ClangdUnitPreambleCallbacks : public