On Mon, Jan 14, 2013 at 12:23 PM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><br><div><div class="im"><div>On Jan 11, 2013, at 2:55 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div><br><blockquote type="cite">
<div class="gmail_quote"><div>There are two different kinds of overflow that can happen in a case expression:</div><div>
<br></div><div>1) An overflow occurs while evaluating the case value:</div><div><br></div><div>switch (n)</div><div>  case 123456 * 123456:</div><div><br></div><div>2) An overflow occurs while converting the evaluated case value to the promoted type of the switch expression:</div>

<div><br></div><div>int n = /*...*/;</div><div>switch (n)</div><div>  case 1ULL << 45:</div><div><br></div><div>For situation (1), it doesn't seem pertinent that this is happening within a case expression; we should warn no matter where it happens; this is the focus of PR2729. (2) should already be being checked by the call to ConvertIntegerToTypeWarnOnOverflow.</div>
</div></blockquote><div><br></div></div>This Patch checks and warns for overflow.</div></div></blockquote><div><br></div><div>Thanks!</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><blockquote type="cite"><div class="gmail_quote"><div>Yes, that would make sense to me, although we will need some mechanism to suppress the warning in the case where the overflow produces an error (as happens in constant expressions in C++11), and we'd need to avoid repeatedly evaluating subexpressions.</div>
</div></blockquote><br></div><div>I am afraid I don't see how  this warning can be avoided in case of constant expressions in C++11. Expression evaluations are disjoint from declarations specs.</div><div>For example, clang already issues this warning (and error for constant expressions ) in the case of left-shift operations. </div>
<div>constexpr int ll = <a href="tel:%282147483647" value="+12147483647" target="_blank">(2147483647</a> << 2);</div><div><br></div><div>Can't we have the additional warning? Please advise.</div></div></blockquote>
<div><br></div><div>I think the additional warning is fine for the time being. In the longer term, we could perform this check in ActOnFinishFullExpr (alongside the call to CheckImplicitConversions), and suppress it if the full expression is required to be a constant expression.</div>
<div><br></div><div>Does this patch have acceptable performance for large constant expressions? I'm a little concerned that it appears it will repeatedly evaluate subexpressions (the constant expression evaluator doesn't perform any caching).</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>- Fariborz</div><div><br></div><div><br></div><div><br><blockquote type="cite">
<div class="gmail_quote">
<div> </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><div><blockquote type="cite"><blockquote type="cite">Also, I changed the warning per your other comments.<br>

In r172102.<br></blockquote><br>Thanks! The "results in a new value" wording seems a bit awkward, how<br>about just saying "overflow in case constant expression results in<br>value %0"?</blockquote></div>

</div></div></blockquote></div>
</blockquote></div><br></div><br></blockquote></div><br>