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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
