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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
