[Lldb-commits] [lldb] r296495 - Fix incorrect logic in StackFrame::Disassemble.

[Zachary Turner via lldb-commits]

That patch was not really about early returns, it was about using
StringRef.  So my comment about the aesthetics was referring to the patch
in general.  The early return was more of a drive by, since I was already
touching the code.  In that respect I was just following the well
documented and widely accepted LLVM policy.

Regarding testing, I honestly don't see a reasonable path forward regarding
increasing test coverage until we have a better testing infrastructure in
place that is less platform-dependent, less flaky, and makes it much much
easier to write small, targeted tests.

On Tue, Feb 28, 2017 at 3:27 PM Jim Ingham <jingham at apple.com> wrote:

>
> > On Feb 28, 2017, at 3:14 PM, Zachary Turner <zturner at google.com> wrote:
> >
> > On Tue, Feb 28, 2017 at 3:07 PM Jason Molenda <jmolenda at apple.com>
> wrote:
> > At it's core, lldb is a real world tool that thousands of people depend
> on; breaking it or introducing bugs for little gain beyond aesthetics is a
> very poor tradeoff.
> >
> > Just for the record, I disagree with this assertion that there is little
> gain beyond aesthetics (as does I think almost everyone else in the
> LLVM/LLDB community).
>
>
> No doubt, early returns makes it easier to reason about complicated code.
> But you added an early return to a function that had maybe 10 lines of code
> in it and was trivial to read either way.  There was pretty much zero
> chance somebody working on the code before this change would introduce a
> bug that they wouldn't because of the clarity provided by the early
> return.  But in doing so you DID add a bug.  In this case it seems clear
> that for the sake of very little more than aesthetics, you introduced a
> bug.  That seems to me a very poor tradeoff.
>
> BTW, somebody at Apple tried to get the llvm version of gcov working on
> the lldb testsuite to see what kind of coverage we actually had.  It didn't
> work right off the bat for reasons that weren't clear, and whoever did the
> initial effort lost the window of time they had to work on this.  But that
> would be a useful exercise; then you could know whether the code you were
> touching was already well tested.  Then we could gate any of these sorts of
> formal manipulations on there being adequate test coverage of the affected
> area in advance of that work.
>
>
> Jim
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170228/e5289069/attachment.html>