changelog shortlog tags branches files raw gz bz2 help

Mercurial > hg > plan9front / changeset: cc: fix wrong "useless or misleading comparison" warning

changeset 6894: 18d51d1c871d
parent 6893: b5e6c7796c9c
child 6895: 8325e5645c93
author: cinap_lenrek@felloff.net
date: Sun, 18 Nov 2018 20:42:45 +0100
files: sys/src/cmd/cc/com.c
description: cc: fix wrong "useless or misleading comparison" warning

to reproduce:

u8int x, y;

x = 0xff;
y = 0xc0;
if((s8int)(x & y) >= 0)
print("help\n");

compiles correctly but prints a warning

warning: test.c:11 useless or misleading comparison: UINT >= 0

the issue is that compar() unconditionally skipped over
all left casts ignoring the case when a cast would sign
extend the value.

the new code only skips over the cast when the original
type with is smaller than the cast result or when they
are equal width and types have same signedness. so the
effective left hand side type is the last truncation
or sign extension.		 
     1.1--- a/sys/src/cmd/cc/com.c
     1.2+++ b/sys/src/cmd/cc/com.c
     1.3@@ -1373,14 +1373,21 @@ compar(Node *n, int reverse)
     1.4 	/*
     1.5 	 * Skip over left casts to find out the original expression range.
     1.6 	 */
     1.7-	while(l->op == OCAST)
     1.8+	while(l->op == OCAST){
     1.9+		lt = l->type;
    1.10+		rt = l->left->type;
    1.11+		if(lt == T || rt == T)
    1.12+			return 0;
    1.13+		if(lt->width < rt->width)
    1.14+			break;
    1.15+		if(lt->width == rt->width && ((lt->etype ^ rt->etype) & 1) != 0)
    1.16+			break;
    1.17 		l = l->left;
    1.18+	}
    1.19 	if(l->op == OCONST)
    1.20 		return 0;
    1.21 	lt = l->type;
    1.22-	if(lt == T)
    1.23-		return 0;
    1.24-	if(lt->etype == TXXX || lt->etype > TUVLONG)
    1.25+	if(lt == T || lt->etype == TXXX || lt->etype > TUVLONG)
    1.26 		return 0;
    1.27 	
    1.28 	/*
    1.29@@ -1399,16 +1406,17 @@ compar(Node *n, int reverse)
    1.30 	if((rt->etype&1) && r->vconst < 0)	/* signed negative */
    1.31 		x.a = ~0ULL;
    1.32 
    1.33-	if((lt->etype&1)==0){
    1.34+	if(lt->etype & 1){
    1.35+		/* signed */
    1.36+		lo = big(~0ULL, -(1LL<<(lt->width*8-1)));
    1.37+		hi = big(0, (1LL<<(lt->width*8-1))-1);
    1.38+	} else {
    1.39 		/* unsigned */
    1.40 		lo = big(0, 0);
    1.41 		if(lt->width == 8)
    1.42 			hi = big(0, ~0ULL);
    1.43 		else
    1.44-			hi = big(0, (1LL<<(l->type->width*8))-1);
    1.45-	}else{
    1.46-		lo = big(~0ULL, -(1LL<<(l->type->width*8-1)));
    1.47-		hi = big(0, (1LL<<(l->type->width*8-1))-1);
    1.48+			hi = big(0, (1LL<<(lt->width*8))-1);
    1.49 	}
    1.50 
    1.51 	switch(op){
    1.52@@ -1462,4 +1470,3 @@ if(debug['y']) prtree(n, "strange");
    1.53 	warn(n, "useless or misleading comparison: %s", cmpbuf);
    1.54 	return 0;
    1.55 }
    1.56-