jansvoboda11 added inline comments.
================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:36
+ llvm::erase_if(Args, [](const std::string &Arg) {
+ return Arg.find("-fmodules-cache-path=") == 0;
+ });
----------------
dexonsmith wrote:
> Is that the only one to remove? (asking because you dropped the FIXME, which
> suggested it might only be one example)
>
> Also, could this just be an assertion, since
> `makeInvocationForModuleBuildWithoutPaths()` is already clearing things?
> - In other words, why do you need this in two places?
> - And if you do need it in both, does the test cover both, or should there be
> another test or RUN line for the other code path?
Ah, there are some others (e.g. `-fmodules-prune-interval=`,
`-fmodules-prune-after=`) so you're right dropping the FIXME might be a bit
hasty. I'll update the fixme instead of removing it.
This cannot be an assertion unfortunately.
`makeInvocationForModuleBuildWithoutPaths()` is clearing the modules cache path
in the **-cc1** command lines for building **modules**. This however, is
dealing with the **driver** command line for building the original
**translation unit**, which doesn't go through the `CompilerInvocation`
machinery at all. We're relying on textual manipulation here at this moment.
(TU command lines are driver command lines that might expand into multiple
commands and therefore are not guaranteed to be representable by single
`CompilerInvocation`.)
The test I touched covers only this change to the driver command line for
building the TU. The error (`-Wunused-command-line-argument`) I test is only
produced by the driver. This means the other change (clearing modules cache
path in -cc1 `CompilerInvocation`) is just a preventive measure, not necessary
to make the updated test pass and not testable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120474/new/
https://reviews.llvm.org/D120474
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
