[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