<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div>On Jun 7, 2013, at 11:35 AM, David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>> wrote:</div><blockquote type="cite">On Friday, June 7, 2013, John McCall wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Jun 2, 2013, at 1:11 AM, David Majnemer <<a href="javascript:;" onclick="_e(event, 'cvml', 'david.majnemer@gmail.com')">david.majnemer@gmail.com</a>> wrote:<br>
> Author: majnemer<br>
> Date: Sun Jun 2 03:11:22 2013<br>
> New Revision: 183084<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=183084&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=183084&view=rev</a><br>
> Log:<br>
> Properly consider the range of enum for range comparisons in C mode<br>
><br>
> In some cases, clang applies the C++ rules for computing the range of a<br>
> value when said value is an enum.<br>
><br>
> Instead, apply C semantics when in C mode.<br>
<br>
Was this proposed or reviewed somewhere? This is totally wrong.</blockquote><div><br></div><div>It was proposed on this list in the form of a patch and LGTM'd. I then waited for more comments and received none.<span></span></div>
</blockquote><br></div><div>Ah, I see it. I'll note that almost all of this waiting happened over the weekend, which is not exactly the peak review period, but okay, I just missed the thread.</div><div><br></div><div>The out-of-range-constant-compare warning should be using a conservatively-wide estimate of the range of the value, not a conservatively-narrow estimate. For example, the analysis is currently based only on types, but if it were instead analyzing expressions, it would want to evaluate the width of adding two chars as 9 bits, not 8.</div><div><br></div><div>There are other warnings, like implicit-truncation warnings, for which the reverse analysis is more appropriate. Those warnings are what most of the IntRange framework are based around.</div><div><br></div><div>It is reasonable for a warning that wants a conservatively-narrow analysis to expect the range of an enum to be just the range of its values. This avoids, e.g., warnings about assigning an enum to a small bit-field.</div><div><br></div><div>It would be reasonable to make the direction of conservatism a parameter of IntRange::forValueOfType — in fact, it would be reasonable to make it a parameter of GetExprRange, although that would require careful inspection of all of that code.</div><div><br></div><div>John.</div>
</body></html>