<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote"><div class="im"><div>I don't think that's the correct solution for many of these cases. There are at least two patterns here:</div>
</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></blockquote><div><br></div><div><br></div><div>Yes, there are some of those - though they're actually the minority of cases in the patch I've sent out.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote"><div></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></blockquote><div><br></div><div>I generally disagree with this kind of "assert with fallback" behavior - it's untested code. An assert is there because we think this is unreachable. If it's reachable then we should have an error handling case & test those failure cases. Otherwise we're just off the map anyway. It's not clear that in all such cases the return after the switch is meaningful/correct/useful in the situation where the default case would've been executed.</div>
<div><br></div><div>But I admit this is a common source of debate about assert usage.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="gmail_quote">
<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></blockquote><div><br></div><div>Sure, I'm happy to apply this transformation in such cases, if any.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="gmail_quote">
<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>
</blockquote></div><br><div>Sure - I'm happy to discuss & apply the variations/better approaches.</div><div><br></div><div>- David</div>