This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Add new CPP arithmetic warning?
- From: Neil Booth <neil at daikokuya dot co dot uk>
- To: gcc at gcc dot gnu dot org
- Cc: Zack Weinberg <zack at codesourcery dot com>
- Date: Thu, 18 Jul 2002 00:01:55 +0100
- Subject: Add new CPP arithmetic warning?
Zack,
I noticed EDG gives a warning about integer promotion for the
following:
#if -1 + 6U
#endif
I can't recall their wording off-hand. I thought this was a good
idea, so I coded it up too. It needed a slight re-work of the
reduction code, which, going through a function pointer, was kind
of awkward anyway. The re-work forms the bulk of the patch, the
code to add the warning is not much in and of itself.
The patch below gives:
$ ./cc1 -E /tmp/foo.c
# 1 "/tmp/foo.c"
# 1 "<built-in>"
# 1 "<command line>"
# 1 "/tmp/foo.c"
/tmp/foo.c:1:12: warning: the left operand of "+" changes sign when
promoted
(and "right operand" if appropriate). The reported location
is not perfect, as the issue is only noticed during stack reduction.
This is hard to get right in the presence of macros.
It is easy to fix and make the reported location the operator itself
(which is what EDG does) when I do line-at-a-time tokens; so I'm
happy to leave this minor issue until then.
Do you agree this warning is worth adding? Should I leave it as an
unconditional warning? It would be nice for the compiler proper to
do something similar too, IMO, though that might be non-trivial.
If you think it's a good idea, I'll post a proper patch with ChangeLog
and minor testsuite updates.
Neil.
Index: cppexp.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cppexp.c,v
retrieving revision 1.126
diff -u -p -r1.126 cppexp.c
--- cppexp.c 17 Jul 2002 17:27:13 -0000 1.126
+++ cppexp.c 17 Jul 2002 22:50:54 -0000
@@ -30,7 +30,8 @@ Boston, MA 02111-1307, USA. */
struct op
{
- cpp_num value; /* The value logically "right" of op. */
+ cpp_num value; /* The value logically "right" of op. */
+ const cpp_token *token; /* The token forming op (for diagnostics). */
enum cpp_ttype op;
};
@@ -52,8 +53,7 @@ static cpp_num num_inequality_op PARAMS
enum cpp_ttype));
static cpp_num num_equality_op PARAMS ((cpp_reader *, cpp_num, cpp_num,
enum cpp_ttype));
-static cpp_num num_mul PARAMS ((cpp_reader *, cpp_num, cpp_num,
- enum cpp_ttype));
+static cpp_num num_mul PARAMS ((cpp_reader *, cpp_num, cpp_num));
static cpp_num num_div_op PARAMS ((cpp_reader *, cpp_num, cpp_num,
enum cpp_ttype));
static cpp_num num_lshift PARAMS ((cpp_num, size_t, size_t));
@@ -65,6 +65,7 @@ static cpp_num eval_token PARAMS ((cpp_r
static struct op *reduce PARAMS ((cpp_reader *, struct op *, enum cpp_ttype));
static unsigned int interpret_float_suffix PARAMS ((const uchar *, size_t));
static unsigned int interpret_int_suffix PARAMS ((const uchar *, size_t));
+static void check_signs PARAMS ((cpp_reader *, const struct op *));
/* Token type abuse to create unary plus and minus operators. */
#define CPP_UPLUS (CPP_LAST_CPP_OP + 1)
@@ -631,60 +632,55 @@ The parser assumes all shifted operators
the flag NO_L_OPERAND is set. These semantics are automatic; any
extra semantics need to be handled with operator-specific code. */
-/* Flags. */
+/* Flags. ALWAYS_EVAL is for operators that should be evaluated even
+ if skip_eval is true. If CHECK_SIGNS, we warn if the effective
+ sign of an operand changes because of integer promotions. */
#define NO_L_OPERAND (1 << 0)
#define LEFT_ASSOC (1 << 1)
+#define ALWAYS_EVAL (1 << 2)
+#define CHECK_SIGNS (1 << 3)
-/* Arity. */
-#define UNARY (1 << 0)
-#define BINARY (1 << 1)
-#define OTHER (1 << 2)
-
-typedef cpp_num (*binary_handler) PARAMS ((cpp_reader *, cpp_num, cpp_num,
- enum cpp_ttype));
/* Operator to priority map. Must be in the same order as the first
N entries of enum cpp_ttype. */
static const struct operator
{
uchar prio;
uchar flags;
- uchar arity;
- binary_handler handler;
} optab[] =
{
- /* EQ */ {0, 0, OTHER, NULL}, /* Shouldn't happen. */
- /* NOT */ {16, NO_L_OPERAND, UNARY, NULL},
- /* GREATER */ {12, LEFT_ASSOC, BINARY, num_inequality_op},
- /* LESS */ {12, LEFT_ASSOC, BINARY, num_inequality_op},
- /* PLUS */ {14, LEFT_ASSOC, BINARY, num_binary_op},
- /* MINUS */ {14, LEFT_ASSOC, BINARY, num_binary_op},
- /* MULT */ {15, LEFT_ASSOC, BINARY, num_mul},
- /* DIV */ {15, LEFT_ASSOC, BINARY, num_div_op},
- /* MOD */ {15, LEFT_ASSOC, BINARY, num_div_op},
- /* AND */ {9, LEFT_ASSOC, BINARY, num_bitwise_op},
- /* OR */ {7, LEFT_ASSOC, BINARY, num_bitwise_op},
- /* XOR */ {8, LEFT_ASSOC, BINARY, num_bitwise_op},
- /* RSHIFT */ {13, LEFT_ASSOC, BINARY, num_binary_op},
- /* LSHIFT */ {13, LEFT_ASSOC, BINARY, num_binary_op},
-
- /* MIN */ {10, LEFT_ASSOC, BINARY, num_binary_op},
- /* MAX */ {10, LEFT_ASSOC, BINARY, num_binary_op},
-
- /* COMPL */ {16, NO_L_OPERAND, UNARY, NULL},
- /* AND_AND */ {6, LEFT_ASSOC, OTHER, NULL},
- /* OR_OR */ {5, LEFT_ASSOC, OTHER, NULL},
- /* QUERY */ {3, 0, OTHER, NULL},
- /* COLON */ {4, LEFT_ASSOC, OTHER, NULL},
- /* COMMA */ {2, LEFT_ASSOC, BINARY, num_binary_op},
- /* OPEN_PAREN */ {1, NO_L_OPERAND, OTHER, NULL},
- /* CLOSE_PAREN */ {0, 0, OTHER, NULL},
- /* EOF */ {0, 0, OTHER, NULL},
- /* EQ_EQ */ {11, LEFT_ASSOC, BINARY, num_equality_op},
- /* NOT_EQ */ {11, LEFT_ASSOC, BINARY, num_equality_op},
- /* GREATER_EQ */ {12, LEFT_ASSOC, BINARY, num_inequality_op},
- /* LESS_EQ */ {12, LEFT_ASSOC, BINARY, num_inequality_op},
- /* UPLUS */ {16, NO_L_OPERAND, UNARY, NULL},
- /* UMINUS */ {16, NO_L_OPERAND, UNARY, NULL}
+ /* EQ */ {0, 0}, /* Shouldn't happen. */
+ /* NOT */ {16, NO_L_OPERAND},
+ /* GREATER */ {12, LEFT_ASSOC | CHECK_SIGNS},
+ /* LESS */ {12, LEFT_ASSOC | CHECK_SIGNS},
+ /* PLUS */ {14, LEFT_ASSOC | CHECK_SIGNS},
+ /* MINUS */ {14, LEFT_ASSOC | CHECK_SIGNS},
+ /* MULT */ {15, LEFT_ASSOC | CHECK_SIGNS},
+ /* DIV */ {15, LEFT_ASSOC | CHECK_SIGNS},
+ /* MOD */ {15, LEFT_ASSOC | CHECK_SIGNS},
+ /* AND */ {9, LEFT_ASSOC | CHECK_SIGNS},
+ /* OR */ {7, LEFT_ASSOC | CHECK_SIGNS},
+ /* XOR */ {8, LEFT_ASSOC | CHECK_SIGNS},
+ /* RSHIFT */ {13, LEFT_ASSOC},
+ /* LSHIFT */ {13, LEFT_ASSOC},
+
+ /* MIN */ {10, LEFT_ASSOC | CHECK_SIGNS},
+ /* MAX */ {10, LEFT_ASSOC | CHECK_SIGNS},
+
+ /* COMPL */ {16, NO_L_OPERAND},
+ /* AND_AND */ {6, LEFT_ASSOC | ALWAYS_EVAL},
+ /* OR_OR */ {5, LEFT_ASSOC | ALWAYS_EVAL},
+ /* QUERY */ {3, ALWAYS_EVAL},
+ /* COLON */ {4, LEFT_ASSOC | ALWAYS_EVAL | CHECK_SIGNS},
+ /* COMMA */ {2, LEFT_ASSOC},
+ /* OPEN_PAREN */ {1, NO_L_OPERAND | ALWAYS_EVAL},
+ /* CLOSE_PAREN */ {0, 0},
+ /* EOF */ {0, 0},
+ /* EQ_EQ */ {11, LEFT_ASSOC},
+ /* NOT_EQ */ {11, LEFT_ASSOC},
+ /* GREATER_EQ */ {12, LEFT_ASSOC | CHECK_SIGNS},
+ /* LESS_EQ */ {12, LEFT_ASSOC | CHECK_SIGNS},
+ /* UPLUS */ {16, NO_L_OPERAND},
+ /* UMINUS */ {16, NO_L_OPERAND}
};
/* Parse and evaluate a C expression, reading from PFILE.
@@ -703,7 +699,6 @@ _cpp_parse_expr (pfile)
cpp_reader *pfile;
{
struct op *top = pfile->op_stack;
- const cpp_token *token = NULL, *prev_token;
unsigned int lex_count;
bool saw_leading_not, want_value = true;
@@ -721,10 +716,9 @@ _cpp_parse_expr (pfile)
{
struct op op;
- prev_token = token;
- token = cpp_get_token (pfile);
lex_count++;
- op.op = token->type;
+ op.token = cpp_get_token (pfile);
+ op.op = op.token->type;
switch (op.op)
{
@@ -736,9 +730,9 @@ _cpp_parse_expr (pfile)
case CPP_HASH:
if (!want_value)
SYNTAX_ERROR2 ("missing binary operator before token \"%s\"",
- cpp_token_as_text (pfile, token));
+ cpp_token_as_text (pfile, op.token));
want_value = false;
- top->value = eval_token (pfile, token);
+ top->value = eval_token (pfile, op.token);
continue;
case CPP_NOT:
@@ -753,15 +747,16 @@ _cpp_parse_expr (pfile)
op.op = CPP_UMINUS;
break;
case CPP_OTHER:
- if (ISGRAPH (token->val.c))
- SYNTAX_ERROR2 ("invalid character '%c' in #if", token->val.c);
+ if (ISGRAPH (op.token->val.c))
+ SYNTAX_ERROR2 ("invalid character '%c' in #if", op.token->val.c);
else
- SYNTAX_ERROR2 ("invalid character '\\%03o' in #if", token->val.c);
+ SYNTAX_ERROR2 ("invalid character '\\%03o' in #if",
+ op.token->val.c);
default:
if ((int) op.op <= (int) CPP_EQ || (int) op.op >= (int) CPP_PLUS_EQ)
SYNTAX_ERROR2 ("token \"%s\" is not valid in preprocessor expressions",
- cpp_token_as_text (pfile, token));
+ cpp_token_as_text (pfile, op.token));
break;
}
@@ -770,7 +765,7 @@ _cpp_parse_expr (pfile)
{
if (!want_value)
SYNTAX_ERROR2 ("missing binary operator before token \"%s\"",
- cpp_token_as_text (pfile, token));
+ cpp_token_as_text (pfile, op.token));
}
else if (want_value)
{
@@ -785,7 +780,7 @@ _cpp_parse_expr (pfile)
SYNTAX_ERROR ("#if with no expression");
if (top->op != CPP_EOF && top->op != CPP_OPEN_PAREN)
SYNTAX_ERROR2 ("operator '%s' has no right operand",
- cpp_token_as_text (pfile, prev_token));
+ cpp_token_as_text (pfile, top->token));
}
top = reduce (pfile, top, op.op);
@@ -826,6 +821,7 @@ _cpp_parse_expr (pfile)
top = _cpp_expand_op_stack (pfile);
top->op = op.op;
+ top->token = op.token;
}
/* The controlling macro expression is only valid if we called lex 3
@@ -870,73 +866,121 @@ reduce (pfile, top, op)
prio = optab[op].prio - ((optab[op].flags & LEFT_ASSOC) != 0);
while (prio < optab[top->op].prio)
{
- if (optab[top->op].arity == UNARY)
- {
- if (!pfile->state.skip_eval)
+ if (!pfile->state.skip_eval && optab[top->op].flags & CHECK_SIGNS)
+ check_signs (pfile, top);
+
+ if (!pfile->state.skip_eval || optab[top->op].flags & ALWAYS_EVAL)
+ switch (top->op)
+ {
+ case CPP_UPLUS:
+ case CPP_UMINUS:
+ case CPP_NOT:
+ case CPP_COMPL:
top[-1].value = num_unary_op (pfile, top->value, top->op);
- top--;
- }
- else if (optab[top->op].arity == BINARY)
- {
- if (!pfile->state.skip_eval)
- top[-1].value = (* (binary_handler) optab[top->op].handler)
- (pfile, top[-1].value, top->value, top->op);
- top--;
- }
- /* Anything changing skip_eval has to be handled here. */
- else switch (top--->op)
- {
- case CPP_OR_OR:
- if (!num_zerop (top->value))
- pfile->state.skip_eval--;
- top->value.low = !num_zerop (top->value) || !num_zerop (top[1].value);
- top->value.high = 0;
- top->value.unsignedp = false;
- top->value.overflow = false;
- break;
+ break;
- case CPP_AND_AND:
- if (num_zerop (top->value))
- pfile->state.skip_eval--;
- top->value.low = !num_zerop (top->value) && !num_zerop (top[1].value);
- top->value.high = 0;
- top->value.unsignedp = false;
- top->value.overflow = false;
- break;
+ case CPP_PLUS:
+ case CPP_MINUS:
+ case CPP_RSHIFT:
+ case CPP_LSHIFT:
+ case CPP_MIN:
+ case CPP_MAX:
+ case CPP_COMMA:
+ top[-1].value = num_binary_op (pfile, top[-1].value,
+ top->value, top->op);
+ break;
- case CPP_OPEN_PAREN:
- if (op != CPP_CLOSE_PAREN)
- {
- cpp_error (pfile, DL_ERROR, "missing ')' in expression");
- return 0;
- }
- top->value = top[1].value;
- return top;
+ case CPP_GREATER:
+ case CPP_LESS:
+ case CPP_GREATER_EQ:
+ case CPP_LESS_EQ:
+ top[-1].value
+ = num_inequality_op (pfile, top[-1].value, top->value, top->op);
+ break;
- case CPP_COLON:
- top--;
- if (!num_zerop (top->value))
- {
+ case CPP_EQ_EQ:
+ case CPP_NOT_EQ:
+ top[-1].value
+ = num_equality_op (pfile, top[-1].value, top->value, top->op);
+ break;
+
+ case CPP_AND:
+ case CPP_OR:
+ case CPP_XOR:
+ top[-1].value
+ = num_bitwise_op (pfile, top[-1].value, top->value, top->op);
+ break;
+
+ case CPP_MULT:
+ top[-1].value = num_mul (pfile, top[-1].value, top->value);
+ break;
+
+ case CPP_DIV:
+ case CPP_MOD:
+ top[-1].value = num_div_op (pfile, top[-1].value,
+ top->value, top->op);
+ break;
+
+ case CPP_OR_OR:
+ top--;
+ if (!num_zerop (top->value))
pfile->state.skip_eval--;
- top->value = top[1].value;
- }
- else
- top->value = top[2].value;
- top->value.unsignedp = (top[1].value.unsignedp
- || top[2].value.unsignedp);
- break;
+ top->value.low = (!num_zerop (top->value)
+ || !num_zerop (top[1].value));
+ top->value.high = 0;
+ top->value.unsignedp = false;
+ top->value.overflow = false;
+ continue;
+
+ case CPP_AND_AND:
+ top--;
+ if (num_zerop (top->value))
+ pfile->state.skip_eval--;
+ top->value.low = (!num_zerop (top->value)
+ && !num_zerop (top[1].value));
+ top->value.high = 0;
+ top->value.unsignedp = false;
+ top->value.overflow = false;
+ continue;
+
+ case CPP_OPEN_PAREN:
+ if (op != CPP_CLOSE_PAREN)
+ {
+ cpp_error (pfile, DL_ERROR, "missing ')' in expression");
+ return 0;
+ }
+ top--;
+ top->value = top[1].value;
+ return top;
+
+ case CPP_COLON:
+ top -= 2;
+ if (!num_zerop (top->value))
+ {
+ pfile->state.skip_eval--;
+ top->value = top[1].value;
+ }
+ else
+ top->value = top[2].value;
+ top->value.unsignedp = (top[1].value.unsignedp
+ || top[2].value.unsignedp);
+ continue;
+
+ case CPP_QUERY:
+ cpp_error (pfile, DL_ERROR, "'?' without following ':'");
+ return 0;
- case CPP_QUERY:
- cpp_error (pfile, DL_ERROR, "'?' without following ':'");
- return 0;
+ default:
+ goto bad_op;
+ }
- default:
- goto bad_op;
+ top--;
+ if (!pfile->state.skip_eval)
+ {
+ if (top->value.overflow)
+ cpp_error (pfile, DL_PEDWARN,
+ "integer overflow in preprocessor expression");
}
-
- if (top->value.overflow && !pfile->state.skip_eval)
- cpp_error (pfile, DL_PEDWARN,
- "integer overflow in preprocessor expression");
}
if (op == CPP_CLOSE_PAREN)
@@ -963,6 +1007,29 @@ _cpp_expand_op_stack (pfile)
return pfile->op_stack + old_size;
}
+/* Emits a warning if the effective sign of either operand of OP
+ changes because of integer promotions. */
+static void
+check_signs (pfile, op)
+ cpp_reader *pfile;
+ const struct op *op;
+{
+ if (op->value.unsignedp == op[-1].value.unsignedp)
+ return;
+
+ if (op->value.unsignedp)
+ {
+ if (!num_positive (op[-1].value, CPP_OPTION (pfile, precision)))
+ cpp_error (pfile, DL_WARNING,
+ "the left operand of \"%s\" changes sign when promoted",
+ cpp_token_as_text (pfile, op->token));
+ }
+ else if (!num_positive (op->value, CPP_OPTION (pfile, precision)))
+ cpp_error (pfile, DL_WARNING,
+ "the right operand of \"%s\" changes sign when promoted",
+ cpp_token_as_text (pfile, op->token));
+}
+
/* Clears the unused high order bits of the number pointed to by PNUM. */
static cpp_num
num_trim (num, precision)
@@ -1383,10 +1450,9 @@ num_part_mul (lhs, rhs)
/* Multiply two preprocessing numbers. */
static cpp_num
-num_mul (pfile, lhs, rhs, op)
+num_mul (pfile, lhs, rhs)
cpp_reader *pfile;
cpp_num lhs, rhs;
- enum cpp_ttype op ATTRIBUTE_UNUSED;
{
cpp_num result, temp;
bool unsignedp = lhs.unsignedp || rhs.unsignedp;