<div dir="ltr">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.<div><br></div><div>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.</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Feb 28, 2017 at 3:27 PM Jim Ingham <<a href="mailto:jingham@apple.com">jingham@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br class="gmail_msg">
> On Feb 28, 2017, at 3:14 PM, Zachary Turner <<a href="mailto:zturner@google.com" class="gmail_msg" target="_blank">zturner@google.com</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
> On Tue, Feb 28, 2017 at 3:07 PM Jason Molenda <<a href="mailto:jmolenda@apple.com" class="gmail_msg" target="_blank">jmolenda@apple.com</a>> wrote:<br class="gmail_msg">
> 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.<br class="gmail_msg">
><br class="gmail_msg">
> 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).<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
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.<br class="gmail_msg">
<br class="gmail_msg">
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.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
Jim<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div>