Thread overview
[dmd-internals] bug 259: error on unsigned/signed comparison
Oct 09, 2011
Christian Kamm
Oct 09, 2011
Robert Clipsham
Oct 10, 2011
Christian Kamm
October 09, 2011
Hi!

I was briefly looking at
http://d.puremagic.com/issues/show_bug.cgi?id=259
today and wondered whether the fix wouldn't be as easy as:

diff --git a/src/cast.c b/src/cast.c
index c7c78fc..b0c77ee 100644
--- a/src/cast.c
+++ b/src/cast.c
@@ -1629,6 +1629,48 @@ int typeMerge(Scope *sc, Expression *e, Type **pt,
Expression **pe1, Expression
         TY ty1 = (TY)Type::impcnvType1[t1b->ty][t2b->ty];
         TY ty2 = (TY)Type::impcnvType2[t1b->ty][t2b->ty];

+        // For compares, don't blindly convert. One range must contain the
other.
+        switch (e->op) {
+        case TOKlt:
+        case TOKle:
+        case TOKgt:
+        case TOKge:
+        case TOKunord:
+        case TOKlg:
+        case TOKleg:
+        case TOKule:
+        case TOKul:
+        case TOKuge:
+        case TOKug:
+        case TOKue: {
+            //printf("\tcompare ranges for comparison\n");
+            IntRange r1 = e1->getIntRange();
+            IntRange r2 = e2->getIntRange();
+            //r1.dump("lhs", e1);
+            //r2.dump("rhs", e2);
+
+            if (IntRange::fromType(t1b).contains(r2)) {
+                //printf("use lhs type\n");
+                t = t1;
+                e2 = e2->castTo(sc, t);
+                goto Lret;
+            }
+            if (IntRange::fromType(t2b).contains(r1)) {
+                //printf("use rhs type\n");
+                t = t2;
+                e1 = e1->castTo(sc, t);
+                goto Lret;
+            }
+
+            //printf("no safe conversion\n");
+            // better error message!
+            goto Lincompatible;
+
+            break;
+        }
+        default: break;
+        }
+
         if (t1b->ty == ty1)     // if no promotions
         {
             if (t1 == t2)

What did I miss? Looking at the first couple of errors when building druntime, they all seem genuine.

One thing is probably C code compatibility. With this change 1u < -1 == false.

Regards,
Christian
October 10, 2011
On 9 October 2011 20:05, Christian Kamm <kamm at incasoftware.de> wrote:

> Hi!
>
> I was briefly looking at
> http://d.puremagic.com/issues/show_bug.cgi?id=259
> today and wondered whether the fix wouldn't be as easy as:
>
> diff --git a/src/cast.c b/src/cast.c
> index c7c78fc..b0c77ee 100644
> --- a/src/cast.c
> +++ b/src/cast.c
> @@ -1629,6 +1629,48 @@ int typeMerge(Scope *sc, Expression *e, Type **pt,
> Expression **pe1, Expression
>         TY ty1 = (TY)Type::impcnvType1[t1b->ty][t2b->ty];
>         TY ty2 = (TY)Type::impcnvType2[t1b->ty][t2b->ty];
>
> +        // For compares, don't blindly convert. One range must contain the
> other.
> +        switch (e->op) {
> +        case TOKlt:
> +        case TOKle:
> +        case TOKgt:
> +        case TOKge:
> +        case TOKunord:
> +        case TOKlg:
> +        case TOKleg:
> +        case TOKule:
> +        case TOKul:
> +        case TOKuge:
> +        case TOKug:
> +        case TOKue: {
> +            //printf("\tcompare ranges for comparison\n");
> +            IntRange r1 = e1->getIntRange();
> +            IntRange r2 = e2->getIntRange();
> +            //r1.dump("lhs", e1);
> +            //r2.dump("rhs", e2);
> +
> +            if (IntRange::fromType(t1b).contains(r2)) {
> +                //printf("use lhs type\n");
> +                t = t1;
> +                e2 = e2->castTo(sc, t);
> +                goto Lret;
> +            }
> +            if (IntRange::fromType(t2b).contains(r1)) {
> +                //printf("use rhs type\n");
> +                t = t2;
> +                e1 = e1->castTo(sc, t);
> +                goto Lret;
> +            }
> +
> +            //printf("no safe conversion\n");
> +            // better error message!
> +            goto Lincompatible;
> +
> +            break;
> +        }
> +        default: break;
> +        }
> +
>         if (t1b->ty == ty1)     // if no promotions
>         {
>             if (t1 == t2)
>
> What did I miss? Looking at the first couple of errors when building
> druntime,
> they all seem genuine.
>
> One thing is probably C code compatibility. With this change 1u < -1 == false.
>
> Regards,
> Christian
>

See also: https://github.com/D-Programming-Language/dmd/pull/119

-- 
Robert
http://octarineparrot.com/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/dmd-internals/attachments/20111010/70f805f0/attachment.html>
October 10, 2011
On Monday 10 October 2011 01:32 Robert Clipsham wrote:
> See also: https://github.com/D-Programming-Language/dmd/pull/119

Oh, I didn't see that one. Fwiw, I cleaned it up (and fixed the C code breakage). It's now https://github.com/D-Programming-Language/dmd/pull/444 .

Christian