[PATCH] D66555: [driver] add a new option `-gen-cdb-fragment-path` to emit a fragment of a compilation database for each compilation

[Alex Lorenz via Phabricator via cfe-commits]

arphaman marked 5 inline comments as done.
arphaman added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2067
+  const auto &Driver = C.getDriver();
+  auto CWD = Driver.getVFS().getCurrentWorkingDirectory();
+  if (!CWD) {
----------------
jkorous wrote:
> Do we still need this now?
No, the CWD should actually be read in `DumpCompilationDatabase`. I updated that method.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2089
+  if (Err) {
+    Driver.Diag(diag::err_drv_compilationdatabase) << Err.message();
+    return;
----------------
jkorous wrote:
> Just to make sure - the error different on purpose here?
Not really needed I guess, I reused the error.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2092
+  }
+  CompilationDatabase =
+      std::make_unique<llvm::raw_fd_ostream>(FD, /*shouldClose=*/true);
----------------
jkorous wrote:
> I'm not familiar with intended lifecycle of `CompilationDatabase ` member. Just to make sure - we need neither to preserve the original value nor reset the stream after we're done here (as in - nobody is going to write anything after we're done here), right?
Yes, we should either preserve or not reset the stream once it's created. Multiple clang invocations can be constructed  from one driver run, so we want to keep the stream open while the driver is running.


================
Comment at: clang/lib/Driver/ToolChains/Clang.h:99
+  void DumpCompilationDatabaseFragmentToDir(
+      StringRef Dir, Compilation &C, StringRef Target, const InputInfo &Output,
+      const InputInfo &Input, const llvm::opt::ArgList &Args) const;
----------------
jkorous wrote:
> BTW this looks kind of funny `const InputInfo &Output, const InputInfo &Input`.
Yes, that's unfortunate type naming in Clang :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66555