Skip to contentSkip to conversation

saner warnings for restricted types

PATCH In review Al Viro 2 participants 5 messages Jan 15, 2008 Sparse checker

lore
Share & tools
lore

Al Viro

author
#0 lore raw

PATCH 1/1 saner warnings for restricted types

* don't crap the type->ident for unsigned int just because somebody did

	typedef unsigned int x;

only structs, unions, enums and restricted types need this logics.
* generate saner warnings for restricted, include type name(s) into them.

---
 evaluate.c   |   33 ++++++++++++++++++++-------------
 parse.c      |   11 +++++++++--
 show-parse.c |    4 ++++
 3 files changed, 33 insertions(+), 15 deletions(-)
evaluate.c +20 -13
index 54fcd3f..777f603 100644
@@ -500,9 +500,10 @@ static inline void unrestrict(struct expression *expr,
 			      int class, struct symbol **ctype)
 {
 	if (class & TYPE_RESTRICT) {
-		warning(expr->pos, "restricted degrades to integer");
 		if (class & TYPE_FOULED)
 			*ctype = unfoul(*ctype);
+		warning(expr->pos, "%sdegrades to integer",
+			show_typename(*ctype));
 		*ctype = (*ctype)->ctype.base_type; /* get to arithmetic type */
 	}
 }
@@ -1235,7 +1236,9 @@ static int evaluate_assign_op(struct expression *expr)
 		}
 		if (tclass & TYPE_RESTRICT) {
 			if (!restricted_binop(op, t)) {
-				expression_error(expr, "bad restricted assignment");
+				warning(expr->pos, "bad assignment (%s) to %s",
+					show_special(op), show_typename(t));
+				expr->right = cast_to(expr->right, target);
 				return 0;
 			}
 			/* allowed assignments unfoul */
@@ -1248,7 +1251,9 @@ static int evaluate_assign_op(struct expression *expr)
 		/* source and target would better be identical restricted */
 		if (t == s)
 			return 1;
-		warning(expr->pos, "invalid restricted assignment");
+		warning(expr->pos, "invalid assignment: %s", show_special(op));
+		info(expr->pos, "   left side has type %s", show_typename(t));
+		info(expr->pos, "   right side has type %s", show_typename(s));
 		expr->right = cast_to(expr->right, target);
 		return 0;
 	}
@@ -1707,10 +1712,8 @@ static struct symbol *evaluate_postop(struct expression *expr)
 		return NULL;
 	}
 
-	if ((class & TYPE_RESTRICT) && restricted_unop(expr->op, &ctype)) {
-		expression_error(expr, "bad operation on restricted");
-		return NULL;
-	}
+	if ((class & TYPE_RESTRICT) && restricted_unop(expr->op, &ctype))
+		return bad_expr_type(expr);
 
 	if (class & TYPE_NUM) {
 		multiply = 1;
@@ -1799,7 +1802,8 @@ static struct symbol *evaluate_preop(struct expression *expr)
 			expr->right->ctype = ctype;
 			expr->right->fvalue = 0;
 		} else if (is_fouled_type(ctype)) {
-			warning(expr->pos, "restricted degrades to integer");
+			warning(expr->pos, "%sdegrades to integer",
+				show_typename(ctype->ctype.base_type));
 		}
 		ctype = &bool_ctype;
 		break;
@@ -2664,9 +2668,11 @@ static struct symbol *evaluate_cast(struct expression *expr)
 
 	if (t1 != t2) {
 		if (class1 & TYPE_RESTRICT)
-			warning(expr->pos, "cast to restricted type");
+			warning(expr->pos, "cast to %s",
+				show_typename(t1));
 		if (class2 & TYPE_RESTRICT)
-			warning(expr->pos, "cast from restricted type");
+			warning(expr->pos, "cast from %s",
+				show_typename(t2));
 	}
 
 	if (t1 == &ulong_ctype)
@@ -3246,9 +3252,10 @@ static void check_case_type(struct expression *switch_expr,
 		return;
 
 	if (!restricted_binop_type(SPECIAL_EQUAL, case_expr, switch_expr,
-				   cclass, sclass, case_type, switch_type))
-		warning(case_expr->pos, "restricted degrades to integer");
-
+				   cclass, sclass, case_type, switch_type)) {
+		unrestrict(case_expr, cclass, &case_type);
+		unrestrict(switch_expr, sclass, &switch_type);
+	}
 	return;
 
 Bad:
parse.c +9 -2
index a41939d..6255737 100644
@@ -2158,8 +2158,15 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
 	base_type = decl->ctype.base_type;
 
 	if (is_typedef) {
-		if (base_type && !base_type->ident)
-			base_type->ident = ident;
+		if (base_type && !base_type->ident) {
+			switch (base_type->type) {
+			case SYM_STRUCT:
+			case SYM_UNION:
+			case SYM_ENUM:
+			case SYM_RESTRICT:
+				base_type->ident = ident;
+			}
+		}
 	} else if (base_type && base_type->type == SYM_FN) {
 		/* K&R argument declaration? */
 		if (lookup_type(token))
show-parse.c +4 -0
index 9a1f796..b9a2828 100644
@@ -309,6 +309,10 @@ static void do_show_type(struct symbol *sym, struct type_name *name,
 		break;
 
 	case SYM_RESTRICT:
+		if (sym->ident) {
+			prepend(name, "restricted %s ", show_ident(sym->ident));
+			return;
+		}
 		break;
 
 	case SYM_FOULED:
Signature · 1 line
1.5.3.GIT
#1 lore raw

evaluate.c static inline void unrestrict(struct expression *expr, · line 506

Quoted 6 lines · Al Viro
evaluate.c +2 -1
 	if (class & TYPE_RESTRICT) {
-		warning(expr->pos, "restricted degrades to integer");
 		if (class & TYPE_FOULED)
 			*ctype = unfoul(*ctype);
+		warning(expr->pos, "%sdegrades to integer",
+			show_typename(*ctype));

Missing space after "%s"?

Signature · 2 lines
Regards,
Pavel Roskin
author
#2 lore raw
Quoted 9 lines · Pavel Roskin

evaluate.c static inline void unrestrict(struct expression *expr, · line 506

Quoted 6 lines · Al Viro
evaluate.c +2 -1
 	if (class & TYPE_RESTRICT) {
-		warning(expr->pos, "restricted degrades to integer");
 		if (class & TYPE_FOULED)
 			*ctype = unfoul(*ctype);
+		warning(expr->pos, "%sdegrades to integer",
+			show_typename(*ctype));

Missing space after "%s"?

No. Try it and you'll see... The way show_typename() (actually, the stuff behind it) works you get a space after the damn thing. What you'll get is something like

kernel/power/main.c:242:2: warning: restricted suspend_state_t degrades to integer

Actually, if you look at the things like

fs/afs/fsclient.c:465:21: warning: incorrect type in assignment (different base types)
fs/afs/fsclient.c:465:21: expected restricted __be32 [usertype] operation_ID
fs/afs/fsclient.c:465:21: got int

you'll see that the last line is "fs/afs/fsclient.c:465:21: got int \n" -
with whitespace in the end. We could get rid of that, but it'll take more massage and it's definitely a separate patch series.

#3 lore raw
Quoted 11 lines · Al Viro
Quoted 9 lines · Pavel Roskin

evaluate.c static inline void unrestrict(struct expression *expr, · line 506

Quoted 6 lines · Al Viro
evaluate.c +2 -1
 	if (class & TYPE_RESTRICT) {
-		warning(expr->pos, "restricted degrades to integer");
 		if (class & TYPE_FOULED)
 			*ctype = unfoul(*ctype);
+		warning(expr->pos, "%sdegrades to integer",
+			show_typename(*ctype));

Missing space after "%s"?

Quoted 1 line

No. Try it and you'll see...

OK, just wanted to make sure. Thanks for the explanation!

Signature · 2 lines
Regards,
Pavel Roskin
author
#4 lore raw
In reply to Al Viro
Quoted 2 lines · Al Viro

with whitespace in the end. We could get rid of that, but it'll take more massage and it's definitely a separate patch series.

Whee...

; cat >foo.c <<'EOF'
int a[2][3];
EOF
; test-parsing foo.c
foo.c:1:5: warning: symbol 'a' was not declared. Should it be static? .align 4
int [addressable] [toplevel] a[3][2]
;

See that [3][2]? IOW, show_typename() really needs fixing. Oh, well...

The problem is that append() part should go *before* recursive call, not after it. We want appended stuff from the outer levels closer to the center (i.e. to the name if it's present). Anyway, the fix follows:

[PATCH] fix show_typename()

* postfix stuff had been added in the wrong order (e.g. int a[2][3] generated int [addressable][toplevel] a[3][2])
* after fixing that, we've no need for recursion anymore, a bunch of arguments go away and turn into local variables and we get an easy way to get rid of bogus space in the show_typename() result.

---
 evaluate.c   |    4 +-
 show-parse.c |  100 +++++++++++++++++++++++++++++----------------------------
 2 files changed, 53 insertions(+), 51 deletions(-)
evaluate.c +2 -2
index 777f603..2901c1b 100644
@@ -502,7 +502,7 @@ static inline void unrestrict(struct expression *expr,
 	if (class & TYPE_RESTRICT) {
 		if (class & TYPE_FOULED)
 			*ctype = unfoul(*ctype);
-		warning(expr->pos, "%sdegrades to integer",
+		warning(expr->pos, "%s degrades to integer",
 			show_typename(*ctype));
 		*ctype = (*ctype)->ctype.base_type; /* get to arithmetic type */
 	}
@@ -1802,7 +1802,7 @@ static struct symbol *evaluate_preop(struct expression *expr)
 			expr->right->ctype = ctype;
 			expr->right->fvalue = 0;
 		} else if (is_fouled_type(ctype)) {
-			warning(expr->pos, "%sdegrades to integer",
+			warning(expr->pos, "%s degrades to integer",
 				show_typename(ctype->ctype.base_type));
 		}
 		ctype = &bool_ctype;
show-parse.c +51 -49

171 lines not on this page · raw message

Signature · 1 line
1.5.3.GIT