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

Dmitri Gribenko via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 6 08:42:50 PDT 2019


gribozavr added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:331
+def index_record_codegen_name : Flag<["-"], "index-record-codegen-name">, Flags<[CC1Option]>,
+  HelpText<"Record the codegen name for symbols">;
+
----------------
Could you add a more detailed description? Even as a compiler engineer, I can't really tell what `index-record-codegen-name` really does or why I would want to turn it on.


================
Comment at: clang/include/clang/Frontend/CompilerInstance.h:180
 
+  /// \brief An optional callback function used to wrap all FrontendActions
+  /// produced to generate imported modules before they are executed.
----------------
No need to write "\brief", it is the default.


================
Comment at: clang/include/clang/Frontend/CompilerInstance.h:180
 
+  /// \brief An optional callback function used to wrap all FrontendActions
+  /// produced to generate imported modules before they are executed.
----------------
gribozavr wrote:
> No need to write "\brief", it is the default.
Please don't write what something is used for, write what something is.

"A factory for wrappers around FrontendActions that compile imported modules."


================
Comment at: clang/include/clang/Frontend/CompilerInstance.h:810
+    return GenModuleActionWrapper;
+  }
+
----------------
I don't see anything actually using `GenModuleActionWrapper`.


================
Comment at: clang/lib/Frontend/CompilerInstance.cpp:1110
 
+  // Pass along the GenModuleActionWrapper callback
+  auto wrapGenModuleAction = ImportingInstance.getGenModuleActionWrapper();
----------------
Please end comments with a period.


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

https://reviews.llvm.org/D64384





More information about the llvm-commits mailing list