Re: [patch] Avoid generating warnings for -O2 -Wunreachable-code

[Richard Guenther <richard dot guenther at gmail dot com>]

On Thu, Nov 5, 2009 at 10:38 AM, Eric Fisher <joefoxreal@gmail.com> wrote:
> 2009/11/4 Richard Guenther <richard.guenther@gmail.com>:
>
>> -Wunreachable-code is seriously broken, I suggest to remove its
>> implementation completely.
>>
>> Richard.
>>
>
> I have no idea about this. At least, we shouldn't count on gcc emiting
> correct warnings of -Wunreachable-code when it does optimization. So
> the testcases should only test flag -Wunreachable-code with -O0. Thus,
> one solution is just to modify the testsuite.
>
> If we plan don't support -Wunreachable-code any more, here is a patch.

The patch should retain -Wunreachable-code as a no-op
for backward compatibility.  And it needs a changelog ;)

The patch is ok to apply in a week if nobody raises serious objections.

Thanks,
Richard.

> Index: doc/invoke.texi
> ===================================================================
> --- doc/invoke.texi ? ? (revision 153929)
> +++ doc/invoke.texi ? ? (working copy)
> @@ -259,7 +259,7 @@ Objective-C and Objective-C++ Dialects}.
> ?-Wstrict-overflow -Wstrict-overflow=@var{n} @gol
> ?-Wswitch ?-Wswitch-default ?-Wswitch-enum -Wsync-nand @gol
> ?-Wsystem-headers ?-Wtrigraphs ?-Wtype-limits ?-Wundef ?-Wuninitialized @gol
> --Wunknown-pragmas ?-Wno-pragmas -Wunreachable-code @gol
> +-Wunknown-pragmas ?-Wno-pragmas @gol
> ?-Wunsuffixed-float-constants ?-Wunused ?-Wunused-function @gol
> ?-Wunused-label ?-Wunused-parameter -Wno-unused-result -Wunused-value
> -Wunused-variable @gol
> ?-Wvariadic-macros -Wvla @gol
> @@ -4188,29 +4188,6 @@ cases where multiple declaration is vali
> ?@opindex Wno-nested-externs
> ?Warn if an @code{extern} declaration is encountered within a function.
>
> -@item -Wunreachable-code
> -@opindex Wunreachable-code
> -@opindex Wno-unreachable-code
> -Warn if the compiler detects that code will never be executed.
> -
> -This option is intended to warn when the compiler detects that at
> -least a whole line of source code will never be executed, because
> -some condition is never satisfied or because it is after a
> -procedure that never returns.
> -
> -It is possible for this option to produce a warning even though there
> -are circumstances under which part of the affected line can be executed,
> -so care should be taken when removing apparently-unreachable code.
> -
> -For instance, when a function is inlined, a warning may mean that the
> -line is unreachable in only one inlined copy of the function.
> -
> -This option is not made part of @option{-Wall} because in a debugging
> -version of a program there is often substantial code which checks
> -correct functioning of the program and is, hopefully, unreachable
> -because the program does work. ?Another common use of unreachable
> -code is to provide behavior which is selectable at compile-time.
> -
> ?@item -Winline
> ?@opindex Winline
> ?@opindex Wno-inline
> Index: testsuite/gcc.dg/Wunreachable-1.c
> ===================================================================
> --- testsuite/gcc.dg/Wunreachable-1.c ? (revision 153929)
> +++ testsuite/gcc.dg/Wunreachable-1.c ? (working copy)
> @@ -1,24 +0,0 @@
> -/* { dg-do compile } */
> -/* { dg-options "-O2 -Wunreachable-code" } */
> -
> -extern void foo (void);
> -extern void baz (void);
> -
> -void bar (int i)
> -{
> - ?if (i < 2)
> - ? ?{
> - ? ? ?baz ();
> - ? ? ?return;
> - ? ?}
> - ?else
> - ? ?{
> - ? ? ?if (i >= 4 && i <= 5)
> - ? ? ? ?foo ();
> - ? ? ?return;
> - ? ?}
> -
> - ?baz (); ? ? ?/* { dg-warning "will never be executed" "" } */
> - ?baz ();
> - ?baz ();
> -}
> Index: testsuite/gcc.dg/Wunreachable-5.c
> ===================================================================
> --- testsuite/gcc.dg/Wunreachable-5.c ? (revision 153929)
> +++ testsuite/gcc.dg/Wunreachable-5.c ? (working copy)
> @@ -1,17 +0,0 @@
> -/* PR c/10175 */
> -
> -/* { dg-do compile } */
> -/* { dg-options "-Wunreachable-code" } */
> -
> -int value;
> -
> -int main(void)
> -{
> - ? ?if (0)
> - ? ? ? ?value = 0; ?/* { dg-warning "will never be executed" "" } */
> - ? ?else
> - ? ? ? ?value = 1;
> -
> - ? ?return 0;
> -}
> -
> Index: testsuite/gcc.dg/Wunreachable-2.c
> ===================================================================
> --- testsuite/gcc.dg/Wunreachable-2.c ? (revision 153929)
> +++ testsuite/gcc.dg/Wunreachable-2.c ? (working copy)
> @@ -1,19 +0,0 @@
> -/* { dg-do compile } */
> -/* { dg-options "-O2 -Wunreachable-code" } */
> -
> -extern int foo (const char *);
> -extern void baz (void);
> -const char *a[] = { "one", "two" };
> -
> -void bar (void)
> -{
> - ?int i;
> -
> - ?for (i = 0; i < 2; i++) ?/* { dg-bogus "will never be executed" ""
> { xfail *-*-* } } */
> - ? ?if (! foo (a[i])) ?/* { dg-bogus "will never be executed" "" {
> xfail *-*-* } } */
> - ? ? ?return;
> -
> - ?baz (); ? ? ?/* { dg-bogus "will never be executed" } */
> - ?baz ();
> - ?baz ();
> -}
> Index: testsuite/gcc.dg/Wunreachable-6.c
> ===================================================================
> --- testsuite/gcc.dg/Wunreachable-6.c ? (revision 153929)
> +++ testsuite/gcc.dg/Wunreachable-6.c ? (working copy)
> @@ -1,21 +0,0 @@
> -/* PR c/11370 ?*/
> -/* { dg-do compile } */
> -/* { dg-options "-Wunreachable-code" } */
> -
> -extern int printf (const char *, ...);
> -extern void exit (int);
> -
> -int main(int argc, char *argv[])
> -{
> - ?if (argc != 1)
> - ? ?exit(1);
> -
> - ?{
> - ? ?int ix; ?/* { dg-bogus "will never be executed" } */
> - ? ?ix = printf("hello\n");
> - ? ?printf("%d\n", ix);
> - ?}
> -
> - ?return 0;
> -}
> -
> Index: testsuite/gcc.dg/pr12603.c
> ===================================================================
> --- testsuite/gcc.dg/pr12603.c ?(revision 153929)
> +++ testsuite/gcc.dg/pr12603.c ?(working copy)
> @@ -1,6 +1,6 @@
> ?/* PR 12603: No return statement warning on function that never
> returns with -O3. */
> ?/* { dg-do compile } */
> -/* { dg-options "-O3 -Wall -Wextra -Wreturn-type -Wunreachable-code" } */
> +/* { dg-options "-O3 -Wall -Wextra -Wreturn-type" } */
> ?int
> ?this_function_never_returns ()
> ?{
> Index: testsuite/gcc.dg/Wunreachable-3.c
> ===================================================================
> --- testsuite/gcc.dg/Wunreachable-3.c ? (revision 153929)
> +++ testsuite/gcc.dg/Wunreachable-3.c ? (working copy)
> @@ -1,17 +0,0 @@
> -/* PR c/10175 */
> -/* { dg-do compile } */
> -/* { dg-options "-Wunreachable-code" } */
> -
> -int i,j;
> -int main(void)
> -{
> - ?if (0) {
> - ? ?i = 0; ? ? ? ? ? ? ? ?/* { dg-warning "will never be executed" "" } */
> - ? ?j = 0;
> - ?} else {
> - ? ?i = 1;
> - ? ?j = 1;
> - ?}
> -
> - ?return 0;
> -}
> Index: testsuite/gcc.dg/Wunreachable-7.c
> ===================================================================
> --- testsuite/gcc.dg/Wunreachable-7.c ? (revision 153929)
> +++ testsuite/gcc.dg/Wunreachable-7.c ? (working copy)
> @@ -1,21 +0,0 @@
> -/* PR c/11370 ?*/
> -/* { dg-do compile } */
> -/* { dg-options "-O2 -Wunreachable-code" } */
> -
> -extern int printf (const char *, ...);
> -extern void exit (int);
> -
> -int main(int argc, char *argv[])
> -{
> - ?if (argc != 1)
> - ? ?exit(1);
> -
> - ?{
> - ? ?int ix; ?/* { dg-bogus "will never be executed" } */
> - ? ?ix = printf("hello\n");
> - ? ?printf("%d\n", ix);
> - ?}
> -
> - ?return 0;
> -}
> -
> Index: testsuite/gcc.dg/20041231-1.c
> ===================================================================
> --- testsuite/gcc.dg/20041231-1.c ? ? ? (revision 153929)
> +++ testsuite/gcc.dg/20041231-1.c ? ? ? (working copy)
> @@ -1,15 +0,0 @@
> -/* PR17544 Incorrect -Wunreachable-code warning
> - ? Origin: Giovanni Bajo
> -
> - ? In C99 we append a "return 0;" when finishing a function, but it was
> - ? not given a source location. ?The gimplifier thinks a return statement
> - ? needs a locus so it would add one, making the compiler generated code
> - ? visible to the unreachable code warning. ?*/
> -
> -/* { dg-do compile } */
> -/* { dg-options "-std=c99 -O -Wunreachable-code" } */
> -
> -int main (void) ? ?// 1
> -{ ? ? ? ? ? ? ? ? ?// 2
> - ?return 0; ? ? ? ?// 3
> -} ? ? ? ? ? ? ? ? ?// 4
> Index: testsuite/gcc.dg/Wunreachable-4.c
> ===================================================================
> --- testsuite/gcc.dg/Wunreachable-4.c ? (revision 153929)
> +++ testsuite/gcc.dg/Wunreachable-4.c ? (working copy)
> @@ -1,12 +0,0 @@
> -/* PR middle-end/10336 */
> -/* { dg-options "-Wunreachable-code" } */
> -
> -void foo(int i)
> -{
> - ?switch(i) {
> - ? ?case 0:
> - ? ? ?break;
> - ? ?case 1:
> - ? ? ?break;
> - ?}
> -}
> Index: testsuite/gcc.dg/Wunreachable-8.c
> ===================================================================
> --- testsuite/gcc.dg/Wunreachable-8.c ? (revision 153929)
> +++ testsuite/gcc.dg/Wunreachable-8.c ? (working copy)
> @@ -1,20 +0,0 @@
> -/* { dg-do compile } */
> -/* { dg-options "-O2 -Wunreachable-code" } */
> -float Factorial(float X)
> -{
> - ?float val = 1.0;
> - ?int k,j;
> - ?for (k=1; k < 5; k++) /* { dg-bogus "will never be executed" "" {
> xfail *-*-* } } */
> - ? ?{
> - ? ? ?val += 1.0; /* { dg-bogus "will never be executed" "" { xfail
> *-*-* } } */
> - ? ?}
> - ?return (val); /* { dg-bogus "will never be executed" } */
> -}
> -
> -int main (void)
> -{
> - ?float result;
> - ?result=Factorial(2.1);
> - ?return (0);
> -}
> -
> Index: common.opt
> ===================================================================
> --- common.opt ?(revision 153929)
> +++ common.opt ?(working copy)
> @@ -200,10 +200,6 @@ Wuninitialized
> ?Common Var(warn_uninitialized) Init(-1) Warning
> ?Warn about uninitialized automatic variables
>
> -Wunreachable-code
> -Common Var(warn_notreached) Warning
> -Warn about code that will never be executed
> -
> ?Wunused
> ?Common Var(warn_unused) Init(0) Warning
> ?Enable all -Wunused- warnings
> Index: tree-cfg.c
> ===================================================================
> --- tree-cfg.c ?(revision 153929)
> +++ tree-cfg.c ?(working copy)
> @@ -1760,7 +1760,6 @@ static void
> ?remove_bb (basic_block bb)
> ?{
> ? gimple_stmt_iterator i;
> - ?source_location loc = UNKNOWN_LOCATION;
>
> ? if (dump_file)
> ? ? {
> @@ -1830,24 +1829,9 @@ remove_bb (basic_block bb)
> ? ? ? ? ? ?i = gsi_last_bb (bb);
> ? ? ? ? ?else
> ? ? ? ? ? ?gsi_prev (&i);
> -
> - ? ? ? ? /* Don't warn for removed gotos. ?Gotos are often removed due to
> - ? ? ? ? ? ?jump threading, thus resulting in bogus warnings. ?Not great,
> - ? ? ? ? ? ?since this way we lose warnings for gotos in the original
> - ? ? ? ? ? ?program that are indeed unreachable. ?*/
> - ? ? ? ? if (gimple_code (stmt) != GIMPLE_GOTO
> - ? ? ? ? ? ? && gimple_has_location (stmt))
> - ? ? ? ? ? loc = gimple_location (stmt);
> ? ? ? ?}
> ? ? }
>
> - ?/* If requested, give a warning that the first statement in the
> - ? ? block is unreachable. ?We walk statements backwards in the
> - ? ? loop above, so the last statement we process is the first statement
> - ? ? in the block. ?*/
> - ?if (loc > BUILTINS_LOCATION && LOCATION_LINE (loc) > 0)
> - ? ?warning_at (loc, OPT_Wunreachable_code, "will never be executed");
> -
> ? remove_phi_nodes_and_edges_for_unreachable_block (bb);
> ? bb->il.gimple = NULL;
> ?}
>