<div class="gmail_quote">On Wed, Sep 21, 2011 at 12:42 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
I'm about to send out a patch to that effect. If someone (Doug, John, etc) wants to just say "yes" when they see that, I'll check it in.</blockquote><div><br></div><div>I don't think that's the correct solution for many of these cases. There are at least two patterns here:</div>
<div><br></div><div>foo bar(...) {</div><div>  switch (x) {</div><div>    default: assert(0 && ...);</div><div>    case ...:</div><div>    case ...:</div><div>    ...</div><div>  }</div><div><br></div><div>  // handle stuff</div>
<div>  return my_foo;</div><div>}</div><div><br></div><div>and</div><div><br></div><div>foo bar(...) {</div><div>  switch (x) {</div><div>    default: assert(0 && ...);</div><div>    case ...: return ...;</div><div>
    case ...: return ...;</div><div>    ...</div><div>  }</div><div>}</div><div><br></div><div>The former should probably stay assert(0 ...). It has some ability to handle that case in opt builds anyways. There is no control flow to prune out.</div>
<div><br></div><div>The latter should probably not have a default *at all*, have a covering set of cases, and have llvm_unreachable *outside the switch*, before the '}', to prevent the missing return from causing a warning.</div>
<div><br></div><div>These are two distinct patterns, and in neither case is blindly replacing assert(0 && ...) the correct transformation. I think each fix needs to be made mindful of the code context it occurs in, and what the reasonable behavior is there.</div>
</div>