[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl added inline comments.


================
Comment at: source/API/SBTarget.cpp:1467
+  }
+  const ConstString csFrom(from), csTo(to);
+  if (csFrom && csTo) {
----------------
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.


https://reviews.llvm.org/D49739