[PATCH] D85077: [clang][Tooling] Optimize addTargetAndMode in case of invalid modes

[Kadir Cetinkaya via Phabricator via cfe-commits]

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