klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG, thx for bearing with me, this looks great.
(and sorry if I didn't send this earlier, just noticed I forgot to hit submit
:( )
Comment at: lib/Tooling/ArgumentsAdjusters.
saugustine updated this revision to Diff 104307.
saugustine added a comment.
Rework this patch to use argument adjusters. It turns out that
the call to newInvocation from ClangFuzzer has a very limited
set of hard-coded arguments, so I don't think it is necessary
to do the hand-adjusting there. An
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 prob
saugustine added a comment.
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 pa
klimek added a comment.
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?
https://reviews.llvm.org/D34304
___
cfe-commits mailing lis
saugustine added a comment.
> Actually, now that I figured out you mean ArgumentAdjusters, I am making
> progress.
Unfortunately, ArgumentAdjusters only work on vector, and while
ToolInvocation::Invocation takes its arguments in that form,
tooling::newInvocation (which returns a CompilerInvoca
saugustine added a comment.
In https://reviews.llvm.org/D34304#785984, @saugustine wrote:
> In https://reviews.llvm.org/D34304#785083, @klimek wrote:
>
> > I think it's cleaner, because it uses a concept we already have (argument
> > adapters).
>
>
> Will you point me to an example of these. Goo
saugustine added a comment.
In https://reviews.llvm.org/D34304#785083, @klimek wrote:
> I think it's cleaner, because it uses a concept we already have (argument
> adapters).
Will you point me to an example of these. Google is coming up empty.
https://reviews.llvm.org/D34304
_
klimek added a comment.
In https://reviews.llvm.org/D34304#784496, @saugustine wrote:
> In https://reviews.llvm.org/D34304#783675, @klimek wrote:
>
> > I think a better way might be to generally leave dependency options alone,
> > add a default argument adapter to filter out all deps related fla
saugustine added a subscriber: klimek.
saugustine added a comment.
In https://reviews.llvm.org/D34304#783675, @klimek wrote:
> I think a better way might be to generally leave dependency options alone,
> add a default argument adapter to filter out all deps related flags, and
> allow users to a
klimek added a comment.
I think a better way might be to generally leave dependency options alone, add
a default argument adapter to filter out all deps related flags, and allow
users to add their own argument adapters that don't do that.
https://reviews.llvm.org/D34304
echristo edited reviewers, added: bkramer; removed: echristo.
echristo added a comment.
Going to let Ben review this. I'd rather not pass the bool down and he might
know a way to avoid that here by knowing the code more :)
https://reviews.llvm.org/D34304
_
12 matches
Mail list logo
