[PATCH] D85077: [clang][Tooling] Optimize addTargetAndMode in case of invalid modes
Haojian Wu via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 3 02:47:08 PDT 2020
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;
----------------
kadircet wrote:
> 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`) ?
I'd not worry too much about the loop, but rename also seems ok to me.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85077/new/
https://reviews.llvm.org/D85077
More information about the cfe-commits
mailing list