jansvoboda11 added a comment. Having this check is great! Left some nits in-line.
================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:644-650 // May perform round-trip of command line arguments. By default, the round-trip // is enabled in assert builds. This can be overwritten at run-time via the // "-round-trip-args" and "-no-round-trip-args" command line flags. // During round-trip, the command line arguments are parsed into a dummy // instance of CompilerInvocation which is used to generate the command line // arguments again. The real CompilerInvocation instance is then created by // parsing the generated arguments, not the original ones. ---------------- The logic is getting a bit complicated here. I think it'd be good to break down what the `CheckAgainstOriginalInvocation` flag does. Feel free to come up with your own description. ================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:746 - // Generate arguments again, this time from the options we will end up using - // for the rest of the compilation. SmallVector<const char *> GeneratedArgs2; + if (CheckAgainstOriginalInvocation) ---------------- This name is now not accurate (along with the comment below). Can you make sure these still make sense with the new logic? ================ Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:658 +#endif + if (RoundTripArgs.getNumOccurrences() ? RoundTripArgs : DoRoundTripDefault) + if (FD.roundTripCommands(llvm::errs())) ---------------- The call to `getNumOccurrences()` is a bit odd. Can you drop that if you add `llvm::cl::init(false)` to the option definition? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143446/new/ https://reviews.llvm.org/D143446 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
