On Tue, Mar 29, 2022 at 12:13:51PM -0400, Marek Polacek wrote:
> This patch fixes a wrong -Wimplicit-fallthrough warning for
> 
>     case 0:
>       if (1)  // wrong may fallthrough
>       return 0;
>     case 1:
> 
> which in .gimple looks like
> 
>     <D.1981>: // case 0
>     if (1 != 0) goto <D.1985>; else goto <D.1986>;
>     <D.1985>:
>     D.1987 = 0;
>     // predicted unlikely by early return (on trees) predictor.
>     return D.1987;
>     <D.1986>:  // dead
>     <D.1982>: // case 1
> 
> and the warning thinks that <D.1986>: falls through to <D.1982>:.  It
> does not know that <D.1986> is effectively a dead label, only reachable
> through fallthrough from previous instructions, never jumped to.  To
> that effect, Jakub introduced UNUSED_LABEL_P, which is set on such dead
> labels.
> 
> collect_fallthrough_labels has code to deal with cases like
> 
>     case 2:
>       if (e != 10)
>       i++; // this may fallthru, warn
>       else
>       return 44;
>     case 3:
> 
> which collects labels that may fall through.  Here it sees the "goto 
> <D.1990>;"
> at the end of the then branch and so when the warning reaches
> 
>     ...
>     <D.1990>: // from if-then
>     <D.1984>: // case 3
> 
> it knows it should warn about the possible fallthrough.  But an UNUSED_LABEL_P
> is not a label that can fallthrough like that, so it should ignore those.
> 
> However, we still want to warn about this:
> 
>     case 0:
>       if (1)
>       n++; // falls through
>     case 1:
> 
> so collect_fallthrough_labels needs to return the "n = n + 1;" statement, 
> rather
> than the dead label.
> 
> Co-authored-by: Jakub Jelinek <ja...@redhat.com>
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
>       PR middle-end/103597
> 
> gcc/ChangeLog:
> 
>       * gimplify.cc (collect_fallthrough_labels): Don't push UNUSED_LABEL_Ps
>       into labels.  Maybe set prev to the statement preceding UNUSED_LABEL_P.
>       (gimplify_cond_expr): Set UNUSED_LABEL_P.
>       * tree.h (UNUSED_LABEL_P): New.
> 
> gcc/testsuite/ChangeLog:
> 
>       * c-c++-common/Wimplicit-fallthrough-39.c: New test.

LGTM.

        Jakub

Reply via email to