kadircet 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;
----------------
hokein wrote:
> 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) {
> ...
> }
> ```
i would rather not duplicate the loop, what about renaming `AlreadyHastTarget`
to `ShouldAddTarget` (likewise for `AlyreadyHasMode`) ?
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
