hokein added inline comments.
================ Comment at: clang/lib/Tooling/Tooling.cpp:264 + // No need to search for target args if we don't have a mode to insert. + bool AlreadyHasTarget = !TargetMode.TargetIsValid; bool AlreadyHasMode = false; ---------------- While this is an optimization, I find the code is a bit harder to follow, with this patch `AlreadyHasTarget` has two semantic meanings: 1) for has target and 2) for target mode is valid. I guess we could do it like ``` if (TargetMode.TargetIsValid) { // set the TargetOPT, TargetOPTLegacy variables // search the command line of the target opt. // insert to CommandLine. } ``` maybe we could do the same thing for the DriverMode ``` if (TargetMode.DriverMode) { ... } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85077/new/ https://reviews.llvm.org/D85077 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits