<div class="gmail_quote">Sorry to come late to this thread, but I'm going to try and provide another perspective here, specifically where Ted asked for other perspectives:</div><div class="gmail_quote"><br></div><div class="gmail_quote">
On Wed, Feb 8, 2012 at 11:02 AM, Ted Kremenek <span dir="ltr"><<a href="mailto:kremenek@apple.com">kremenek@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 class="im"><div>A fair point.  I think it's worth pointing out that a fair number of people disliked this change, so much that they requested that we could put the inverse case under a different warning flag so they could turn it off.</div>
</div></div><div><br></div><div>I'm leaning that this should be under -Wall after all, but it would be great to hear other opinions here.</div></div></blockquote><div><br></div><div> I think the problem with this warning under -Wall is this: there are competing coding patterns.</div>
<div><br></div><div>With -Wparentheses, by luck, history, or whatever, we have gotten the overwhelming majority of code using the same pattern: (x == y) and ((x = y)). That means we get to warn very easily on both sides of this pattern.</div>
<div><br></div><div>With this warning we have gotten wonderful consistency of switches-over-enums *without* default cases, but there remain many patterns with defaults:</div><div><br></div><div>switch (x) {</div><div>  default:</div>
<div>  case X:</div><div>  ...</div><div>}</div><div><br></div><div>switch (x) {</div><div>  default: abort();</div><div>  ...</div><div>}</div><div><br></div><div>switch (x) {</div><div>  default: assert(0 && ...);</div>
<div>  ...</div><div>}</div><div><br></div><div><br></div><div>While we have some good evidence that there is a better pattern (the one this warning pushes people toward), but there is a considerable amount of code that currently follows one of the old patterns.</div>
<div><br></div><div>To me, the question of whether we should try to drive code toward a single style through a -Wall should be answered by looking at how buggy the existing patterns are, and how frequently they are followed. For code I've looked at, this warning occurs fairly frequently, and has found few or no bugs. For code I've looked at, the converse pattern added to -Wparentheses fired fairly infrequently, and in a reasonable number of cases found bugs.</div>
<div><br></div><div>Given that, I think -Wcovered-switch-default is useful to folks who are willing to switch coding patterns to one we can defend against future bugs, but I fear it has too high of a "falso positive" from people who are satisfied with finding related bugs via 'abort()' or 'assert()' or whatever to place under -Wall.</div>
<div><br></div><div>I think we should have two categories here, with better names:</div><div><br></div><div>-Wimportant -- These are warnings we expect to be valuable even for large existing codebases. We recommend essentially everyone that can use these.</div>
<div><br></div><div>-Wuseful -- These are warnings which have high utility, but may require non-trivial code changes to accomodate.</div><div><br></div><div>There are probably still better names, these are what I came up with off the cuff.</div>
<div><br></div><div>Currently we use "-Wall" to mean the first of these. I wonder if we could use -Wextra to mean the second. I also wonder whether we should add aliases with more clear names.</div><div><br></div>
<div>-Chandler</div><div><br></div><div><br></div></div>