[PATCH] D137534: [C++20] [Modules] [ClangScanDeps] Allow clang-scan-deps to without specified compilation database in P1689 (3/4)

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.


================
Comment at: clang/test/ClangScanDeps/P1689.cppm:9
+//
+// Check the seperated dependency format.
+// RUN: clang-scan-deps -format=p1689 --p1689-targeted-file-name=%t/M.cppm --p1689-targeted-output=%t/M.o \
----------------
ben.boeckel wrote:
> jansvoboda11 wrote:
> > What does "separated" mean in this context?
> Yeah, this isn't the right term. There are two things being done:
> 
> - discovering dependencies for a future compile (P1689)
> - collecting deps for the scanning itself to know that "if included file X changes, I need to rescan"
> 
> Both are required for correct builds.
@ChuanqiXu Can you write up a better comment here?


================
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:571-581
+
+  // FIXME: createInvocation will drop the `-o` option since it requires
+  // `-fsyntax-only`. So here we try to parse the output file manually.
+  auto CommandLineIter =
+      std::find(CommandLine.rbegin(), CommandLine.rend(), StringRef("-o"));
+  if (CommandLineIter == CommandLine.rend() ||
+      --CommandLineIter == CommandLine.rend()) {
----------------
ChuanqiXu wrote:
> ChuanqiXu wrote:
> > Here is the corresponding code: https://github.com/llvm/llvm-project/blob/0e11d65a58da32311b562ecea2b5ba9d4d655659/clang/lib/Frontend/CreateInvocationFromCommandLine.cpp#L39-L43. There is another FIXME.
> > 
> > Simply, we will lost the output file by using `createInvocation` and I feel it is not easy to fix. Then I feel it is not too bad to parse `-o` manually here if we document it clearly.  I understand the current style will miss some cases like using `-output=`, `-output` instead of `-o` or missing `-o`. But I don't feel too bad for it.
> `createInvocation` will be helpful to recognize many cases, e.g., `-MF` in https://reviews.llvm.org/D139168. I feel it'll be worse if we don't use `createInvocation `.
This is again doing ad-hoc parsing of the command-line. `clang-cl` accepts output file path in the `/o` argument, which this code will not recognize. You should be able to use `CompilerInvocation::CreateFromArgs()` to avoid having the `-fsyntax-only` option injected.


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

https://reviews.llvm.org/D137534