Relational comparison with bitwise results is always true/false

[Richard Smith]

On Mon, Aug 25, 2014 at 2:23 PM, Richard Trieu <rtrieu at google.com> wrote:

> I brought up the same question last year.  By including this in the CFG
> builder, the other analysis that use the CFG can use the tautological
> results for better diagnostics.
>
>
> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131028/091857.html
>

Interesting; I'm sorry I missed that thread...

I agree with a lot of the arguments, but I am not persuaded by the
conclusion. We should not require a CFG to be built in order to analyze
non-control-flow-dependent properties of expressions; that puts the cart
before the horse. And there's more problems muddled up in here -- our
expression evaluator also has a 'try to evaluate this using any tricks you
like' mode, which gives us a *third* way of checking for tautological
comparisons.

I think we need to take a step back and design this all properly. It looks
like a good approach would have been to factor out the code for computing
these 'known' comparison results into some common layer that could be used
by the evaluator, by SemaChecking, and when building the CFG. In fact,
perhaps the best home for them would simply be within the expression
evaluator; the CFG builder *already* calls it in these cases, but then it
does some additional "patch up" work in these special cases. SemaChecking's
-Wtautological-compare could then evaluate the comparison, and diagnose if
the result is known (with appropriate smarts for picking the right
diagnostic message).

Thoughts?

On Mon, Aug 25, 2014 at 12:30 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Mon, Aug 25, 2014 at 9:19 AM, Anders Rönnholm <
>> Anders.Ronnholm at evidente.se> wrote:
>>
>>> Hi,
>>>
>>> I have made a new bitwise comparison always true check for relational
>>> comparisons i'd like to get reviewed.
>>>
>>> e.g
>>> if ((x & 64) < 65) // bitwise comparison always evaluates to true
>>
>>
>> I'm very surprised to see this being implemented in Analysis/CFG.cpp;
>> this belongs in Sema/SemaChecking.cpp. It makes no sense to me to couple
>> this check to building a CFG.
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140825/5e5ebe3a/attachment.html>