shivanshu3 marked 5 inline comments as done.
shivanshu3 added inline comments.
================
Comment at: clang/lib/Tooling/ArgumentsAdjusters.cpp:117
+ // do not remove those when using the cl driver.
+ bool IsDependencyFileArg;
+ if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
----------------
hans wrote:
> Instead of a bool and if below, I'd suggest if-statements and early continues
> instead. Breaking up the old if-statement is nice though, and maybe the
> comment from above about what flags to ignore could be moved down here to
> those if statements. For example:
>
>
> ```
> // -M flags blah blah
> if (Arg.startswith("-M") && !UsingClDriver)
> continue;
> // MSVC flags blah blah
> if (Arg.startswith("/showIncludes") || Arg.startswith("-showIncludes"))
> continue;
> AdjustedArgs.push_back(Args[i]);
> ```
I realized that with my original change, we would skip the next argument under
cl driver mode when -MT was used, which would be wrong. The next argument
should only be skipped when `IsDependencyFileArg` is true. So I think it might
be cleaner to keep that extra boolean so the code is easy to read and
understand. What do you think?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86999/new/
https://reviews.llvm.org/D86999
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
