apolyakov added inline comments.
================
Comment at: source/API/SBTarget.cpp:1467
+ }
+ const ConstString csFrom(from), csTo(to);
+ if (csFrom && csTo) {
----------------
aprantl wrote:
> apolyakov wrote:
> > aprantl wrote:
> > > personally I would write this as:
> > > ```
> > > if (!csFrom)
> > > return error.SetErrorString("<from> path is empty");
> > > if (!csTo)
> > > return error.SetErrorString("<to> path is empty");
> > > Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_API));
> > > if (log)
> > > log->Printf("SBTarget(%p)::%s: '%s' -> '%s'",
> > > static_cast<void *>(target_sp.get()), __FUNCTION__,
> > > from, to);
> > > target_sp->GetImageSearchPathList().Append(csFrom, csTo, true);
> > > ```
> > in my opinion, the branch `if (csFrom && csTo)` will be executed in most
> > cases, so it is reasonable to make it executed first, as this will benefit
> > the branch prediction of the processor. Is this a worthwhile argument?
> Not really. First, you don't actually control the order in which the code is
> emitted by the compiler by rearranging the source code. Second, it doesn't
> make sense to think about branch prediction outside of a tight loop as the
> effect would not even be measurable. We should optimize for readability and
> compact code here.
Yes, I should agree that thinking about branch prediction isn't so effective in
our case. But the readability of our approaches is almost equal. At the same
time, taking into consideration that in most cases we will have not empty
string arguments, your approach will check 2 conditions and mine approach will
check 1 condition.
https://reviews.llvm.org/D49739
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
