[PATCH] D34304: Allow CompilerInvocations to generate .d files.

Manuel Klimek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 23 01:51:52 PDT 2017


klimek added a comment.

In https://reviews.llvm.org/D34304#788080, @saugustine wrote:

> In https://reviews.llvm.org/D34304#787699, @klimek wrote:
>
> > I mean, arguments need to be adjusted before converting to ArgStringList and calling newInvocation? I'm not sure I fully understand the problem, can you elaborate?
>
>
> This gets back to why the original patch plumbed the boolean all the way down to newInvocation.
>
> newInvocation is the function that actually discards the dependency file options--today unconditionally.


Correct, and it shouldn't.

> But there is code that calls newInvocation directly (ClangFuzzer is one), without going through a higher-level API. So I can't adjust the arguments at a higher level and still preserve the old behavior.

Can't the call sites that use it themselves fix the arguments themselves? We should be able to provide a convenience wrapper for those use cases, I'd guess they all look basically the same?

> Unfortunately, newInvocation's argument list type is incompatible with ArgumentAdjusters, so something else will need to be done. What do you recommend?

I'd have expected something like:

1. create ArgumentAdjuster that filters out deps file creation args
2. make that the default in the higher level APIs that already use ArgumentAdjuster
3. for each call site that uses the lower level API, look into what data they have (probably just argv from main?) and create some nice ArgumentAdjuster wrapper so they can use the default argument adjuster with a 1 line change


https://reviews.llvm.org/D34304





More information about the cfe-commits mailing list