[PATCH] D86999: getClangStripDependencyFileAdjuster(): Do not remove -M args when using MSVC cl driver

[Shivanshu Goyal via Phabricator via llvm-commits]

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