tberghammer added a comment.
In http://reviews.llvm.org/D16769#340936, @clayborg wrote:
> 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.
I think if this is not the format we want then we should check clang-format to
support our use case. In this specific case I think putting everything in 1
line is fine as you see the function arguments when you step in and I also
think clang-format will do something different for constructors with more code
in them.
> - 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.
I made these changes intentionally because I feel that inlining these
temporaries improve the readability as you don't have to match the very similar
6-7 letter words by eye and it shouldn't hurt debugging too much because you
can either evaluate the expression you are interested or just display "*this"
or "rhs" as a frame variable. From efficiency perspective they won't matter if
the types used are basic types (PODs) but if the compiler can optimize them out
if they have a non-trivial constructor/destructor. I can revert those if you
want.
http://reviews.llvm.org/D16769
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
