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

[Greg Clayton via lldb-commits]

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:

  bool
  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:

  bool
  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.


http://reviews.llvm.org/D16769