aprantl added inline comments.
================
Comment at: source/API/SBTarget.cpp:1467
+ }
+ const ConstString csFrom(from), csTo(to);
+ if (csFrom && csTo) {
----------------
apolyakov wrote:
> 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.
It's the LLVM style to prefer early exits:
http://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code
https://reviews.llvm.org/D49739
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
