From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7283 invoked by alias); 3 May 2003 21:56:00 -0000 Mailing-List: contact gcc-prs-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-prs-owner@gcc.gnu.org Received: (qmail 7270 invoked by uid 71); 3 May 2003 21:56:00 -0000 Date: Sat, 03 May 2003 21:56:00 -0000 Message-ID: <20030503215600.7269.qmail@sources.redhat.com> To: zack@gcc.gnu.org Cc: gcc-prs@gcc.gnu.org, From: Zack Weinberg Subject: Re: c/10604: -Wall includes sign conversion warning [3.3 regression] Reply-To: Zack Weinberg X-SW-Source: 2003-05/txt/msg00212.txt.bz2 List-Id: The following reply was made to PR c/10604; it has been noted by GNATS. From: Zack Weinberg 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 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 ): 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- Warn if an object is larger than 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" } */ +}