[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

2019-08-29 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr marked 2 inline comments as done.
gribozavr added a comment.

Committed as r370337.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66878/new/

https://reviews.llvm.org/D66878



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


[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

2019-08-28 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov accepted this revision.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang-tools-extra/clangd/index/IndexAction.cpp:217
   std::unique_ptr Includes;
+  index::IndexingOptions Opts;
   std::unique_ptr PragmaHandler;

gribozavr wrote:
> ilya-biryukov wrote:
> > Are these option ever used? Do we need to keep them alive for the lifetime 
> > of the action?
> > Might be worth a comment.
> They are passed in through the constructor, and consumed by 
> `index::createIndexingASTConsumer` in `CreateASTConsumer`. So they need to be 
> stored in a member variable between those two calls.
Ah, missed that at first glance. Thanks for the explanation, LG.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66878/new/

https://reviews.llvm.org/D66878



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


[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

2019-08-28 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr marked 4 inline comments as done.
gribozavr added inline comments.



Comment at: clang-tools-extra/clangd/index/IndexAction.cpp:217
   std::unique_ptr Includes;
+  index::IndexingOptions Opts;
   std::unique_ptr PragmaHandler;

ilya-biryukov wrote:
> Are these option ever used? Do we need to keep them alive for the lifetime of 
> the action?
> Might be worth a comment.
They are passed in through the constructor, and consumed by 
`index::createIndexingASTConsumer` in `CreateASTConsumer`. So they need to be 
stored in a member variable between those two calls.



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:227
+  std::shared_ptr DataConsumer;
+  index::IndexingOptions Opts;
   CommentHandler *PragmaHandler;

ilya-biryukov wrote:
> Same here, we do not seem to use `Opts`, but still store them. To keep them 
> alive?
Same as in the other comment -- `Opts` are passed in through the constructor of 
`IndexAction`, and consumed in `IndexAction::CreateASTConsumer`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66878/new/

https://reviews.llvm.org/D66878



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


[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

2019-08-28 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Overall LG, thanks! Not sure why we need to keep `IndexingOptions` everywhere, 
though, see the relevant comment.




Comment at: clang-tools-extra/clangd/index/IndexAction.cpp:217
   std::unique_ptr Includes;
+  index::IndexingOptions Opts;
   std::unique_ptr PragmaHandler;

Are these option ever used? Do we need to keep them alive for the lifetime of 
the action?
Might be worth a comment.



Comment at: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp:227
+  std::shared_ptr DataConsumer;
+  index::IndexingOptions Opts;
   CommentHandler *PragmaHandler;

Same here, we do not seem to use `Opts`, but still store them. To keep them 
alive?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66878/new/

https://reviews.llvm.org/D66878



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


[PATCH] D66878: [Index] Stopped wrapping FrontendActions in libIndex and its users

2019-08-28 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr created this revision.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous.
Herald added a project: clang.
gribozavr added reviewers: ilya-biryukov, jkorous.
Herald added a subscriber: dexonsmith.
gribozavr added a parent revision: D66877: Moved the IndexDataConsumer::finish 
call into the IndexASTConsumer from IndexAction.
gribozavr added a child revision: D66879: [Index] Added a 
ShouldSkipFunctionBody callback to libIndex, and refactored clients to use it 
instead of inventing their own solution.

Exposed a new function, createIndexingASTConsumer, that creates an
ASTConsumer. ASTConsumers compose well.

Removed wrapping functionality from createIndexingAction.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66878

Files:
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
  clang/include/clang/Index/IndexingAction.h
  clang/lib/Index/IndexingAction.cpp
  clang/tools/c-index-test/core_main.cpp
  clang/tools/libclang/Indexing.cpp

Index: clang/tools/libclang/Indexing.cpp
===
--- clang/tools/libclang/Indexing.cpp
+++ clang/tools/libclang/Indexing.cpp
@@ -19,6 +19,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/MultiplexConsumer.h"
 #include "clang/Frontend/Utils.h"
 #include "clang/Index/IndexingAction.h"
 #include "clang/Lex/HeaderSearch.h"
@@ -367,14 +368,16 @@
 
 class IndexingFrontendAction : public ASTFrontendAction {
   std::shared_ptr DataConsumer;
+  IndexingOptions Opts;
 
   SharedParsedRegionsStorage *SKData;
   std::unique_ptr ParsedLocsTracker;
 
 public:
   IndexingFrontendAction(std::shared_ptr dataConsumer,
+ const IndexingOptions ,
  SharedParsedRegionsStorage *skData)
-  : DataConsumer(std::move(dataConsumer)), SKData(skData) {}
+  : DataConsumer(std::move(dataConsumer)), Opts(Opts), SKData(skData) {}
 
   std::unique_ptr CreateASTConsumer(CompilerInstance ,
  StringRef InFile) override {
@@ -398,8 +401,12 @@
   std::make_unique(*SKData, *PPRec, PP);
 }
 
-return std::make_unique(*DataConsumer,
-  ParsedLocsTracker.get());
+std::vector> Consumers;
+Consumers.push_back(std::make_unique(
+*DataConsumer, ParsedLocsTracker.get()));
+Consumers.push_back(
+createIndexingASTConsumer(DataConsumer, Opts, CI.getPreprocessorPtr()));
+return std::make_unique(std::move(Consumers));
   }
 
   TranslationUnitKind getTranslationUnitKind() override {
@@ -569,12 +576,9 @@
   auto DataConsumer =
 std::make_shared(client_data, CB, index_options,
   CXTU->getTU());
-  auto InterAction = std::make_unique(DataConsumer,
- SkipBodies ? IdxSession->SkipBodyData.get() : nullptr);
-  std::unique_ptr IndexAction;
-  IndexAction = createIndexingAction(DataConsumer,
-getIndexingOptionsFromCXOptions(index_options),
- std::move(InterAction));
+  auto IndexAction = std::make_unique(
+  DataConsumer, getIndexingOptionsFromCXOptions(index_options),
+  SkipBodies ? IdxSession->SkipBodyData.get() : nullptr);
 
   // Recover resources if we crash before exiting this method.
   llvm::CrashRecoveryContextCleanupRegistrar
@@ -995,4 +999,3 @@
   *static_cast(location.ptr_data[0]);
   return cxloc::translateSourceLocation(DataConsumer.getASTContext(), Loc);
 }
-
Index: clang/tools/c-index-test/core_main.cpp
===
--- clang/tools/c-index-test/core_main.cpp
+++ clang/tools/c-index-test/core_main.cpp
@@ -221,9 +221,8 @@
   auto DataConsumer = std::make_shared(OS);
   IndexingOptions IndexOpts;
   IndexOpts.IndexFunctionLocals = indexLocals;
-  std::unique_ptr IndexAction;
-  IndexAction = createIndexingAction(DataConsumer, IndexOpts,
- /*WrappedAction=*/nullptr);
+  std::unique_ptr IndexAction =
+  createIndexingAction(DataConsumer, IndexOpts);
 
   auto PCHContainerOps = std::make_shared();
   std::unique_ptr Unit(ASTUnit::LoadFromCompilerInvocationAction(
Index: clang/lib/Index/IndexingAction.cpp
===
--- clang/lib/Index/IndexingAction.cpp
+++ clang/lib/Index/IndexingAction.cpp
@@ -23,39 +23,7 @@
 
 namespace {
 
-class IndexASTConsumer : public ASTConsumer {
-  std::shared_ptr PP;
-  std::shared_ptr IndexCtx;
-
-public:
-  IndexASTConsumer(std::shared_ptr PP,
-   std::shared_ptr IndexCtx)
-  : PP(std::move(PP)), IndexCtx(std::move(IndexCtx)) {}
-
-protected:
-  void Initialize(ASTContext ) override {
-