[PATCH] D122385: [clang][deps] Fix clang-cl output argument parsing

[Sylvain Audi via Phabricator via cfe-commits]

saudi added a comment.

Nice! It's much more straightforward now, the reverse iteration was pretty confusing.
Thanks!



================
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:478
+              if (Arg == "-Xclang")
+                ++I;
+
----------------
A test for boundary (`I+1 != FlagsEnd`) will be needed here, to avoid an infinite loop in the case `-Xclang` is the last arg.

I think it would be worth doing `continue` here, both because `Arg`'s value is `-Xclang`, which won't be used in this iteration, and also it is a bit confusing as `Arg` has now represents `*(I-1)` instead of `*I`





================
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:483
+              // Also, clang-cl adds ".obj" extension if none is found.
+              llvm::StringRef CurrentO;
+              if ((Arg == "/o" || Arg == "-o") && I != FlagsEnd)
----------------
`llvm::` is unnecessary here, as it's not used for other `StringRef` in this file


================
Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:484
+              llvm::StringRef CurrentO;
+              if ((Arg == "/o" || Arg == "-o") && I != FlagsEnd)
+                CurrentO = *++I;
----------------
The test for the end should be on `I+1`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122385/new/

https://reviews.llvm.org/D122385