Bug 259 Comparing signed to unsigned does not generate an error

[lionello]

http://d.puremagic.com/issues/show_bug.cgi?id=259

Consider a comparison a < b, a <= b, a > b, or a >= b, in which a and b are
integral types of different signedness. Without loss of generality, let's
consider a is signed and b is unsigned and the comparison is a < b. Then we
have the following cases:

1. If a.sizeof > b.sizeof, then a < b is lowered into a < cast(typeof(a)) b.
   Then signed comparison proceeds normally. This is a classic value-based
   conversion dating from the C days, and we do it in D as well.
2. If the sizeof common type of a and b is bigger than both sizeof(a) and sizeof(b), both a and b can be safely cast to the bigger [signed] type and the comparison proceeds normally.
3. Otherwise, if a is determined through Value Range Propagation to be greater
   than or equal to zero, then a < b is lowered into cast(U) a < b, where U is the
   unsigned variant of typeof(a). Then unsigned comparison proceeds normally.
4. Otherwise, the comparison is in error.

Using deprecation() since warnings and errors during template instantiations are suppressed and the final "errors instantiating template" doesn't say why ( http://d.puremagic.com/issues/show_bug.cgi?id=9960 )

[lionello]

This one is blocked by bug 9960

[lionello]

Added test cases. These confirm that we're not changing the outcome of the comparisons (for example -1>2UL), but only introducing deprecation warnings/errors.

[lionello]

Will need to fix all issues in druntime (phobos is built with -d, strangely enough) before this can pass the tests.

dlang/druntime#601

[lionello]

@donc @andralex Please check!

[lionello]

The last commit adds an IntRange variable to VarDeclaration, which can be used to limit the range of mutable variables, in the case where the range can be proven to be limited. Settings this range for the key in ForEachRangeStatement limits the number of false positives.

This can be made to work for general for/while loops as well, further lowering the false positives. Although, it could be seen as a reason to use ForEachRange over hand written loops.

Implementing getIntRange for VarDeclaration has numerous optimization advantages, unrelated to bug 259, so perhaps it's worth pulling this out into its own PR.

TODO: must ensure the foreach body doesn't mutate the iterator.

[lionello]

Rebased on top of my if-else-range branch, which should cut down on false positives.

[andralex]

This is good work that has fallen into oblivion. @lionello could you please rebase and let's put it back on the docket. Thanks!

[John-Colvin]

This would be lovely to have...

[tsbockman]

I have created a version of this patch which is updated for DDMD. See it here: https://github.com/tsbockman/dmd/tree/issue_259

It basically works, except that it causes some false positives for unreachable code detection. For example:

module main;

void main(string[] args)
{
    int* a = null;
    int* c = new int;

    for (auto b = a; true; b++)
    {
        if (b > c)
            break;

        return; // DDMD with my patch incorrectly flags this line as unreachable.
    }
}

It seems to be limited to for loops with pointer arithmetic. So, typical idiomatic D code is unaffected - but it really messes up druntime.

Perhaps @lionello or @John-Colvin could take a look at my work, and point me in the right direction?

I did my translation from C++ into D somewhat blind, without fully understanding the context for the original changes by @lionello, so I really don't know if I messed up somehow, or the bug was already there, or if perhaps it is caused by other changes made to the compiler since June 2014.

[tsbockman]

I figured out what was causing the bug above, but in the process I have decided to rework much of the patch, anyway, to address various other problems I found.

[lionello]

Superseded by #5229 . Many thanks to @tsbockman for porting. Let's work together to get this in!

[tsbockman]

@lionello It might help move things along if you would review my work in #5229.

A simple "LGTM" comment would suffice, if you don't find any problems.

[lionello]

@tsbockman I will! But right now it's 1am and I'm off to bed.