[Lldb-commits] [PATCH] D16769: [NFC] Cleanup RangeMap.h

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Mon Feb 1 11:44:53 PST 2016

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Looks fine. A few things I don't like, but I can live with:

- Making constructors appear all on one line does save space, but it means you can't debug very well as stepping into these constructors will stop on the constructor line and then if you step, you will step  over and out of the entire thing. Granted these classes are very simple and probably don't require debugging anymore, but it is a drawback to clang-format.
- Many functions had variables that were put into temporaries like:

  DoesAdjoinOrIntersect (const Range &rhs) const
      const BaseType lhs_base = this->GetRangeBase();
      const BaseType rhs_base = rhs.GetRangeBase();
      const BaseType lhs_end = this->GetRangeEnd();
      const BaseType rhs_end = rhs.GetRangeEnd();
      bool result = (lhs_base <= rhs_end) && (lhs_end >= rhs_base);
      return result;

were reduced to lines like the following:

  DoesAdjoinOrIntersect(const Range &rhs) const
      return GetRangeBase() <= rhs.GetRangeEnd() && GetRangeEnd() >= rhs.GetRangeBase();

So I see the efficiency, but if I step into this function now it means less useful debugging as you will be stepping into and out of functions and you will lose the values and not be able to see what is going on. The compiler will get rid of the temporary variables when it optimizes things so I am not sure why these changes make things better.


More information about the lldb-commits mailing list