This is the mail archive of the
gcc-prs@gcc.gnu.org
mailing list for the GCC project.
Re: c/10604: -Wall includes sign conversion warning [3.3regression]
- From: Zack Weinberg <zack at codesourcery dot com>
- To: zack at gcc dot gnu dot org
- Cc: gcc-prs at gcc dot gnu dot org,
- Date: 3 May 2003 21:56:00 -0000
- Subject: Re: c/10604: -Wall includes sign conversion warning [3.3regression]
- Reply-to: Zack Weinberg <zack at codesourcery dot com>
The following reply was made to PR c/10604; it has been noted by GNATS.
From: Zack Weinberg <zack@codesourcery.com>
To: ak@suse.de
Cc: gcc-gnats@gcc.gnu.org, gcc-patches@gcc.gnu.org
Subject: Re: c/10604: -Wall includes sign conversion warning [3.3
regression]
Date: Sat, 03 May 2003 14:46:34 -0700
Zack Weinberg <zack@codesourcery.com> writes:
> It looks like this was experimentally changed as part of the C
> option-parsing rewrite; since it clearly causes problems, it should be
> put back the way it was. I am testing the appended patch; Mark has
> approved it for 3.3 if it works. (The patch for 3.4 will be slightly
> more invasive, because I'm going to get rid of a historical wart at
> the same time; that will come later.)
Here's the patch for 3.4. The historical wart is that several
front-end warnings looked at (warn_something || extra_warnings) when
it really should be that -Wextra toggles those bits too.
Unfortunately, implementing it was harder than it should've been.
Down with independent_decode_option.
There is a behavior change with this patch: -Wextra now enables
-Wsign-compare for C++ and ObjC as well as plain C. I cannot imagine
this was anything other than an oversight.
The Makefile.in change is not strictly related, but I needed it to
bootstrap, and it's pretty obvious - flex output may yield warnings
that we can't easily get rid of, so turn off -Werror for it.
zw
PR c/10604
* c-common.c (warn_sign_compare): Initialize to -1.
* c-opts.c (c_common_init_options): Don't set warn_sign_compare here.
(c_common_decode_option <OPT_Wall>): Set warn_sign_compare
for C++ only.
(c_common_post_options): Set warn_sign_compare from extra_warnings
if it's still -1 at this point.
* toplev.c (maybe_warn_unused_parameter): New static variable.
(set_Wextra): New static function.
(W_options): Remove "extra".
(decode_W_option): Call set_Wextra.
(independent_decode_option): Likewise.
(set_Wunused): Cooperate with set_Wextra in setting
warn_unused_parameter.
(rest_of_compilation): No need to check extra_warnings as
well as warn_uninitialized.
* c-typeck.c (build_binary_op, build_conditional_expr):
No need to check extra_warnings as well as warn_sign_compare.
(internal_build_compound_expr): No need to check extra_warnings
as well as warn_unused_value.
* function.c (expand_function_end): No need to check extra_warnings
as well as warn_unused_parameter.
* stmt.c (expand_expr_stmt_value): No need to check extra_warnings
as well as warn_unused_value.
* cp/typeck.c (build_x_compound_expr): No need to check
extra_warnings as well as warn_unused_value.
* doc/invoke.texi: Clarify documentation of -Wsign-compare.
* gcc.dg/compare7.c, g++.dg/warn/compare1.C: New testcases.
* Makefile.in: Disable -Werror for gengtype-lex.o.
===================================================================
Index: Makefile.in
--- Makefile.in 3 May 2003 05:43:34 -0000 1.1046
+++ Makefile.in 3 May 2003 21:31:15 -0000
@@ -165,6 +165,8 @@ insn-conditions.o-warn = -Wno-error
# Bison-1.75 output often yields (harmless) -Wtraditional warnings
gengtype-yacc.o-warn = -Wno-error
c-parse.o-warn = -Wno-error
+# flex output may yield harmless "no previous prototype" warnings
+gengtype-lex.o-warn = -Wno-error
# All warnings have to be shut off in stage1 if the compiler used then
# isn't gcc; configure determines that. WARN_CFLAGS will be either
===================================================================
Index: c-common.c
--- c-common.c 3 May 2003 13:28:29 -0000 1.413
+++ c-common.c 3 May 2003 21:31:17 -0000
@@ -304,9 +304,10 @@ int warn_parentheses;
int warn_missing_braces;
/* Warn about comparison of signed and unsigned values.
- If -1, neither -Wsign-compare nor -Wno-sign-compare has been specified. */
+ If -1, neither -Wsign-compare nor -Wno-sign-compare has been specified
+ (in which case -Wextra gets to decide). */
-int warn_sign_compare;
+int warn_sign_compare = -1;
/* Nonzero means warn about usage of long long when `-pedantic'. */
===================================================================
Index: c-opts.c
--- c-opts.c 1 May 2003 16:13:27 -0000 1.41
+++ c-opts.c 3 May 2003 21:31:17 -0000
@@ -595,8 +595,6 @@ c_common_init_options (lang)
flag_const_strings = (lang == clk_cplusplus);
warn_pointer_arith = (lang == clk_cplusplus);
- if (lang == clk_c)
- warn_sign_compare = -1;
}
/* Handle one command-line option in (argc, argv).
@@ -805,7 +803,8 @@ c_common_decode_option (argc, argv)
warn_parentheses = on;
warn_return_type = on;
warn_sequence_point = on; /* Was C only. */
- warn_sign_compare = on; /* Was C++ only. */
+ if (c_language == clk_cplusplus)
+ warn_sign_compare = on;
warn_switch = on;
warn_strict_aliasing = on;
@@ -1525,6 +1524,11 @@ c_common_post_options (pfilename)
flag_inline_functions = 0;
}
}
+
+ /* -Wextra implies -Wsign-compare, but not if explicitly
+ overridden. */
+ if (warn_sign_compare == -1)
+ warn_sign_compare = extra_warnings;
/* Special format checking options don't work without -Wformat; warn if
they are used. */
===================================================================
Index: c-typeck.c
--- c-typeck.c 30 Apr 2003 01:28:34 -0000 1.235
+++ c-typeck.c 3 May 2003 21:31:18 -0000
@@ -2458,8 +2458,7 @@ build_binary_op (code, orig_op0, orig_op
converted = 1;
resultcode = xresultcode;
- if ((warn_sign_compare < 0 ? extra_warnings : warn_sign_compare != 0)
- && skip_evaluation == 0)
+ if (warn_sign_compare && skip_evaluation == 0)
{
int op0_signed = ! TREE_UNSIGNED (TREE_TYPE (orig_op0));
int op1_signed = ! TREE_UNSIGNED (TREE_TYPE (orig_op1));
@@ -3448,8 +3447,7 @@ build_conditional_expr (ifexp, op1, op2)
and later code won't know it used to be different.
Do this check on the original types, so that explicit casts
will be considered, but default promotions won't. */
- if ((warn_sign_compare < 0 ? extra_warnings : warn_sign_compare)
- && !skip_evaluation)
+ if (warn_sign_compare && !skip_evaluation)
{
int unsigned_op1 = TREE_UNSIGNED (TREE_TYPE (orig_op1));
int unsigned_op2 = TREE_UNSIGNED (TREE_TYPE (orig_op2));
@@ -3603,7 +3601,7 @@ internal_build_compound_expr (list, firs
/* The left-hand operand of a comma expression is like an expression
statement: with -Wextra or -Wunused, we should warn if it doesn't have
any side-effects, unless it was explicitly cast to (void). */
- if ((extra_warnings || warn_unused_value)
+ if (warn_unused_value
&& ! (TREE_CODE (TREE_VALUE (list)) == CONVERT_EXPR
&& VOID_TYPE_P (TREE_TYPE (TREE_VALUE (list)))))
warning ("left-hand operand of comma expression has no effect");
===================================================================
Index: function.c
--- function.c 2 May 2003 14:22:09 -0000 1.423
+++ function.c 3 May 2003 21:31:20 -0000
@@ -6956,13 +6956,8 @@ expand_function_end (filename, line, end
}
}
- /* Warn about unused parms if extra warnings were specified. */
- /* Either ``-Wextra -Wunused'' or ``-Wunused-parameter'' enables this
- warning. WARN_UNUSED_PARAMETER is negative when set by
- -Wunused. Note that -Wall implies -Wunused, so ``-Wall -Wextra'' will
- also give these warnings. */
- if (warn_unused_parameter > 0
- || (warn_unused_parameter < 0 && extra_warnings))
+ /* Possibly warn about unused parameters. */
+ if (warn_unused_parameter)
{
tree decl;
===================================================================
Index: stmt.c
--- stmt.c 20 Apr 2003 22:58:28 -0000 1.299
+++ stmt.c 3 May 2003 21:31:22 -0000
@@ -2174,7 +2174,7 @@ expand_expr_stmt_value (exp, want_value,
{
if (! TREE_SIDE_EFFECTS (exp))
{
- if ((extra_warnings || warn_unused_value)
+ if (warn_unused_value
&& !(TREE_CODE (exp) == CONVERT_EXPR
&& VOID_TYPE_P (TREE_TYPE (exp))))
warning_with_file_and_line (emit_filename, emit_lineno,
===================================================================
Index: toplev.c
--- toplev.c 3 May 2003 13:28:32 -0000 1.750
+++ toplev.c 3 May 2003 21:31:23 -0000
@@ -124,6 +124,7 @@ static int decode_f_option PARAMS ((cons
static int decode_W_option PARAMS ((const char *));
static int decode_g_option PARAMS ((const char *));
static unsigned int independent_decode_option PARAMS ((int, char **));
+static void set_Wextra PARAMS ((int));
static void print_version PARAMS ((FILE *, const char *));
static int print_single_switch PARAMS ((FILE *, int, int, const char *,
@@ -1463,6 +1464,9 @@ int warn_unused_parameter;
int warn_unused_variable;
int warn_unused_value;
+/* Used for cooperation between set_Wunused and set_Wextra. */
+static int maybe_warn_unused_parameter;
+
/* Nonzero to warn about code which is never reached. */
int warn_notreached;
@@ -1586,8 +1590,6 @@ static const lang_independent_options W_
N_("Warn when an optimization pass is disabled") },
{"deprecated-declarations", &warn_deprecated_decl, 1,
N_("Warn about uses of __attribute__((deprecated)) declarations") },
- {"extra", &extra_warnings, 1,
- N_("Print extra (possibly unwanted) warnings") },
{"missing-noreturn", &warn_missing_noreturn, 1,
N_("Warn about functions which might be candidates for attribute noreturn") },
{"strict-aliasing", &warn_strict_aliasing, 1,
@@ -1600,17 +1602,34 @@ set_Wunused (setting)
{
warn_unused_function = setting;
warn_unused_label = setting;
- /* Unused function parameter warnings are reported when either ``-W
- -Wunused'' or ``-Wunused-parameter'' is specified. Differentiate
- -Wunused by setting WARN_UNUSED_PARAMETER to -1. */
- if (!setting)
- warn_unused_parameter = 0;
- else if (!warn_unused_parameter)
- warn_unused_parameter = -1;
+ /* Unused function parameter warnings are reported when either
+ ``-Wextra -Wunused'' or ``-Wunused-parameter'' is specified.
+ Thus, if -Wextra has already been seen, set warn_unused_parameter;
+ otherwise set maybe_warn_extra_parameter, which will be picked up
+ by set_Wextra. */
+ maybe_warn_unused_parameter = setting;
+ warn_unused_parameter = (setting && extra_warnings);
warn_unused_variable = setting;
warn_unused_value = setting;
}
+static void
+set_Wextra (setting)
+ int setting;
+{
+ extra_warnings = setting;
+ warn_unused_value = setting;
+ warn_unused_parameter = (setting && maybe_warn_unused_parameter);
+
+ /* We save the value of warn_uninitialized, since if they put
+ -Wuninitialized on the command line, we need to generate a
+ warning about not using it without also specifying -O. */
+ if (setting == 0)
+ warn_uninitialized = 0;
+ else if (warn_uninitialized != 1)
+ warn_uninitialized = 2;
+}
+
/* The following routines are useful in setting all the flags that
-ffast-math and -fno-fast-math imply. */
@@ -3206,7 +3225,7 @@ rest_of_compilation (decl)
| (flag_thread_jumps ? CLEANUP_THREADING : 0));
timevar_pop (TV_FLOW);
- if (warn_uninitialized || extra_warnings)
+ if (warn_uninitialized)
{
uninitialized_vars_warning (DECL_INITIAL (decl));
if (extra_warnings)
@@ -3874,6 +3893,7 @@ display_help ()
W_options[i].string, _(description));
}
+ printf (_(" -Wextra Print extra (possibly unwanted) warnings\n"));
printf (_(" -Wunused Enable unused warnings\n"));
printf (_(" -Wlarger-than-<number> Warn if an object is larger than <number> bytes\n"));
printf (_(" -p Enable function profiling\n"));
@@ -4256,11 +4276,11 @@ decode_W_option (arg)
}
else if (!strcmp (arg, "extra"))
{
- /* We save the value of warn_uninitialized, since if they put
- -Wuninitialized on the command line, we need to generate a
- warning about not using it without also specifying -O. */
- if (warn_uninitialized != 1)
- warn_uninitialized = 2;
+ set_Wextra (1);
+ }
+ else if (!strcmp (arg, "no-extra"))
+ {
+ set_Wextra (0);
}
else
return 0;
@@ -4539,15 +4559,9 @@ independent_decode_option (argc, argv)
break;
case 'W':
+ /* For backward compatibility, -W is the same as -Wextra. */
if (arg[1] == 0)
- {
- extra_warnings = 1;
- /* We save the value of warn_uninitialized, since if they put
- -Wuninitialized on the command line, we need to generate a
- warning about not using it without also specifying -O. */
- if (warn_uninitialized != 1)
- warn_uninitialized = 2;
- }
+ set_Wextra (1);
else
return decode_W_option (arg + 1);
break;
===================================================================
Index: cp/typeck.c
--- cp/typeck.c 22 Apr 2003 05:44:11 -0000 1.457
+++ cp/typeck.c 3 May 2003 21:31:25 -0000
@@ -4793,7 +4793,7 @@ build_x_compound_expr (list)
/* the left-hand operand of a comma expression is like an expression
statement: we should warn if it doesn't have any side-effects,
unless it was explicitly cast to (void). */
- if ((extra_warnings || warn_unused_value)
+ if (warn_unused_value
&& !(TREE_CODE (TREE_VALUE(list)) == CONVERT_EXPR
&& VOID_TYPE_P (TREE_TYPE (TREE_VALUE(list)))))
warning("left-hand operand of comma expression has no effect");
===================================================================
Index: testsuite/g++.dg/warn/compare1.C
--- testsuite/g++.dg/warn/compare1.C 1 Jan 1970 00:00:00 -0000
+++ testsuite/g++.dg/warn/compare1.C 3 May 2003 21:31:26 -0000
@@ -0,0 +1,10 @@
+/* -Wall is supposed to trigger -Wsign-compare for C++. PR 10604.
+ See also gcc.dg/compare7.c. */
+
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+int f(unsigned a, int b)
+{
+ return a < b; /* { dg-warning "signed and unsigned" } */
+}
===================================================================
Index: testsuite/gcc.dg/compare7.c
--- testsuite/gcc.dg/compare7.c 1 Jan 1970 00:00:00 -0000
+++ testsuite/gcc.dg/compare7.c 3 May 2003 21:31:27 -0000
@@ -0,0 +1,10 @@
+/* -Wall is not supposed to trigger -Wsign-compare for C. PR 10604.
+ See also g++.dg/warn/compare1.C. */
+
+/* { dg-do compile } */
+/* { dg-options "-Wall" } */
+
+int f(unsigned a, int b)
+{
+ return a < b; /* { dg-bogus "signed and unsigned" } */
+}