<div dir="ltr">Just when you thought I was going to let this thread die! :)<div><br></div><div style><div style="font-family:arial,sans-serif;font-size:13px">I *still* feel like I have not seen justification for changing the compiler in this way to merit the possibility of breaking working code. The only argument so far is that this change enables optimization, and I think that argument is full of holes:</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">#1. The optimization only applies to code that warns with -Wreturn-type. For a developer to resolve that, removing -Wreturn-type would be a very bad idea, so the right resolution would be to use #pragma diagnostic. Those changes would involve more code than just marking the function in a way to declare intent, which would enable the compiler to do the optimization without this change.</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px"><div>#2. In other situations, we very commonly argue that it is ok to change behavior "because we have a warning for it". Some of the arguments on this thread are based on "this makes sense if developers ignore warnings in case XXX", but that is most certainly not something we want to encourage.</div>
<div><br></div></div><div style="font-family:arial,sans-serif;font-size:13px">#3. The optimization, in my opinion, would have little real positive impact on the code examples I have seen.</div><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px">#4. The optimization can have very unpredictable and hard to debug behavior on code that currently works. We could resolve this by always trapping, but that defeats most of the optimization point.</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">#5. I think having divergent behavior here between C and C++ is going to be unexpected for users.</div>
<div><br></div><div>John is the code owner here, so at the end of the day his choice wins, but the underlying theme that I disagree with very much is the general disregard for how stability in the compiler is beneficial for users. Naturally I am not arguing that we should never change things that could break code, but I strongly disagree with what I perceive as an attitude that we shouldn't bother to evaluate such choices.</div>
<div style><br></div><div style> - Daniel</div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jan 23, 2013 at 5:42 PM, Daniel Dunbar <span dir="ltr"><<a href="mailto:daniel@zuster.org" target="_blank">daniel@zuster.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Wed, Jan 23, 2013 at 5:18 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div><div>On Jan 23, 2013, at 4:51 PM, Daniel Dunbar <<a href="mailto:daniel@zuster.org" target="_blank">daniel@zuster.org</a>> wrote:</div>
<blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jan 23, 2013 at 4:43 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div>On Jan 23, 2013, at 4:28 PM, Daniel Dunbar <<a href="mailto:daniel@zuster.org" target="_blank">daniel@zuster.org</a>> wrote:</div>
<blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jan 23, 2013 at 4:22 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div>
On Jan 23, 2013, at 4:08 PM, Daniel Dunbar <<a href="mailto:daniel@zuster.org" target="_blank">daniel@zuster.org</a>> wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
On Wed, Jan 23, 2013 at 2:17 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>
<div>On Jan 23, 2013, at 12:42 PM, Daniel Dunbar <<a href="mailto:daniel@zuster.org" target="_blank">daniel@zuster.org</a>> wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">
On Wed, Jan 23, 2013 at 12:08 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>On Jan 23, 2013, at 11:57 AM, Daniel Dunbar <<a href="mailto:daniel@zuster.org" target="_blank">daniel@zuster.org</a>> wrote:</div>
<blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jan 23, 2013 at 11:44 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div><br><div class="gmail_quote">
On Wed, Jan 23, 2013 at 11:07 AM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">A significant part of the problem, I believe, is that there's a lot of mostly-externally-maintained C code which, at Apple, happens to need to be compiled as C++.</blockquote>
</div><br></div>FWIW, this makes perfect sense, and also makes perfect sense out of a flag to essentially get C's return semantics in a C++ compilation in order to support such code.</div></div></blockquote><div><br>
</div><div>This is still the wrong direction of the flag. I just haven't seen good justification for changing the compiler in this way to merit the possibility of breaking working code.</div></div></div></div></blockquote>
<br></div><div>Every change can break working code. Warning changes can break working code if it's compiled with -Werror. "Show me a whole-percentage speedup or take the optimization out" is not really a reasonable response to every last proposal.</div>
</div></blockquote><div><br></div><div>Yes, but that doesn't mean such changes should be made without consideration either. My argument is that I do not think there is sufficient user benefit to motivate this change.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div> In LLVM and clang, we have a lot of places where we use unreachable annotations; I think Chandler's argument is quite correct that these situations come up all the time for many users and that it's ultimately not reasonable to expect non-compiler people to use those annotations pervasively.</div>
<div><br></div><div>Our specific internal problem that makes this seem like a non-starter is that we have a pool of known code that's very awkward to fix. We do control the build environment for that code, though. For purposes of investigation, we can reasonably assume that any project that turns off -Wreturn-value should probably also disable the optimization. Any stragglers can be tracked down and fixed just like we would with any other compiler change.</div>
</div></blockquote><div><br></div><div>You are hijacking my argument. My opinion doesn't have anything to do with an internal problem, I just happen to think this is the wrong choice for users.</div></div></div></div>
</blockquote><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>
</div><div>In my opinion, for *most* users and most code it is more important that the code work than that it be optimal. I think this is the kind of optimization that compiler hackers and low-level optimization people might find very desirable, but anyone writing code that depended on it should still be using an attribute or other marker.</div>
<div><br></div><div>Again in my opinion, for most users, the compiler is just a tool they use to get work done. They like it to optimize, and they like it to give nice warnings, but overall they want it to help them get work done and not force them to change their code.</div>
</div></div></div></blockquote><div><br></div></div>I do not think that this is a reasonable standard by which we can judge optimizations. The compiler is a tool. Like most tools, it can do good things but is capable of mischief. Like most tools, users come to take the good things for granted, but they notice the mischief immediately. It's wrong to forsake the good just because it's less visible; you have to actually make a thoughtful decision to balance these things, and that means not immediately throwing out all the good the first time you see the bad.</div>
</div></blockquote><div><br></div><div>By these standards, what is the good that this change is making?</div><div><br></div><div>I stand by what I said before -- <span style="font-family:arial,sans-serif;font-size:13px">I still have not seen justification for changing the compiler in this way to merit the possibility of breaking working code.</span></div>
<div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">The only justification that has been offered so far is that this change can help the compiler optimize somewhat better ***only in the case of code that would emit a compiler warning***.</span></div>
</div></div></div></blockquote><div><br></div></div>A user can audit their code, decide that the end of a function is in practice unreachable, and take appropriate measures — they might disable that warning, either file-wide or with a #pragma, or they might simply choose to ignore it, knowing that it's a false positive.</div>
</div></blockquote><div><br></div><div>Are you seriously proposing this is the "good thing"? I feel this is just arguing to win not arguing for what is good for the user.</div></div></div></div></blockquote><div>
<br></div></div></div>No. The good thing is that the compiler can optimize somewhat better.</div></blockquote><div><br></div><div>Are there convincing examples that this is really a useful optimization in general?</div>
</div></div></div></blockquote><div><br></div></div></div>Well, I've listed a whole bunch of situations that seem fairly convincing to me, but I've only described them in prose, so I guess I need to write them out so that you'll acknowledge them.<br>
<div><br></div><div> const char *getOrderString(NSComparisonResult result) {</div><div> switch (thing) {</div><div> case NSOrderedAscending: return "less than";</div><div> case NSOrderedSame: return "same as";</div>
<div> case NSOrderedDescending: return "greater than";</div><div> }</div><div> }</div><div><br></div><div> const char *describeThisInt(int x) {</div><div> if (x > 100) return "it's really big!";</div>
<div> if (x > 10) return "it's not that small";</div><div> if (x > 0) return "it's pretty small";</div><div> assert(0 && "A non-positive int?! What is up with that???");</div>
<div> // ^ won't warn in debug builds on most platforms</div><div> }</div></div></div></blockquote><div><br></div></div></div><div>Yes, but these seem like pretty limited examples of optimization. The savings here are either a few instructions of presumed dead code or one perfectly predicted branch.</div>
<div><br></div><div> - Daniel</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span><font color="#888888"><div>
John.</div></font></span></div></div></blockquote></div><br></div></div>
</blockquote></div><br></div>