[PATCH] D64384: [WIP] Index-while-building

[Dmitri Gribenko via Phabricator via llvm-commits]

gribozavr added inline comments.


================
Comment at: clang/include/clang/Frontend/FrontendOptions.h:391
+  std::string IndexStorePath;
+  unsigned IndexIgnoreSystemSymbols : 1;
+  unsigned IndexRecordCodegenName : 1;
----------------
Could we change "ignore system symbols" into a positive option ("record system symbols")? Throughout the patch please.


================
Comment at: clang/include/clang/Index/CodegenNameGenerator.h:46
   struct Implementation;
-  std::unique_ptr<ASTNameGenerator> Impl;
+  Implementation *Impl;
 };
----------------
Why change to a raw pointer?

We can use a unique_ptr to an incomplete type, as long as the destructor `~CodegenNameGenerator` is defined in a file where the type is complete.


================
Comment at: clang/lib/Index/CodegenNameGenerator.cpp:31
+
+  bool writeName(const Decl *D, raw_ostream &OS) {
+    // First apply frontend mangling.
----------------
This code looks like a straight copy from `clang/lib/AST/Mangle.cpp` -- can it be de-duplicated?


================
Comment at: clang/unittests/Index/IndexTests.cpp:74
     AST = &Ctx;
-    IndexDataConsumer::initialize(Ctx);
   }
----------------
This change can be a separate patch.


================
Comment at: clang/unittests/Index/IndexTests.cpp:106
 
+  virtual void setPreprocessor(std::shared_ptr<Preprocessor> PP) override {}
+  virtual bool handleModuleOccurence(const ImportDecl *ImportD,
----------------
Unclear why this test overrides these methods, if the implementation is identical to the base class.


================
Comment at: clang/unittests/Index/IndexTests.cpp:112
+  }
+  virtual void finish() override {}
+
----------------
No need to add `virtual` when we already have `override`.


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

https://reviews.llvm.org/D64384