On Wed, 24 Nov 2021, Jason Merrill wrote:
> On 11/24/21 11:15, Marek Polacek wrote:
> > On Wed, Nov 24, 2021 at 04:21:31PM +0100, Richard Biener via Gcc-patches
> > wrote:
> >> This resurrects -Wunreachable-code and implements a warning for
> >> trivially unreachable code as of CFG construction. Most problematic
> >> with this is the C/C++ frontend added 'return 0;' stmt in main
> >> which the patch handles for C++ like the C frontend already does
> >> by using BUILTINS_LOCATION.
> >>
> >> Another problem for future enhancement is that after CFG construction
> >> we no longer can point to the stmt making a stmt unreachable, so
> >> this implementation tries to warn on the first unreachable
> >> statement of a region. It might be possible to retain a pointer
> >> to the stmt that triggered creation of a basic-block but I'm not
> >> sure how reliable that would be.
> >>
> >> So this is really a simple attempt for now, triggered by myself
> >> running into such a coding error. As always, the perfect is the
> >> enemy of the good.
> >>
> >> It does not pass bootstrap (which enables -Wextra), because of the
> >> situation in g++.dg/Wunreachable-code-5.C where the C++ frontend
> >> prematurely elides conditions like if (! GATHER_STATISTICS) that
> >> evaluate to true - oddly enough it does _not_ do this for
> >> conditions evaluating to false ... (one of the
> >> c-c++-common/Wunreachable-code-2.c cases).
> >
> > I've taken a look into the C++ thing. This is genericize_if_stmt:
> > if we have
> >
> > if (0)
> > return;
> >
> > then cond is integer_zerop, then_ is a return_expr, but since it has
> > TREE_SIDE_EFFECTS, we create a COND_EXPR. For
> >
> > if (!0)
> > return;
> >
> > we do
> > 170 else if (integer_nonzerop (cond) && !TREE_SIDE_EFFECTS (else_))
> > 171 stmt = then_;
> > which elides the if completely.
> >
> > So it seems it would help if we avoided eliding the if stmt if
> > -Wunreachable-code is in effect. I'd be happy to make that change,
> > if it sounds sane.
Yes, that seems to work.
> Sure.
>
> Currently the front end does various constant folding as part of
> genericization, as I recall because there were missed optimizations without
> it. Is this particular one undesirable because it's at the statement level
> rather than within an expression?
It's undesirable because it short-circuits control flow and thus
if (0)
return;
foo ();
becomes
return;
foo ();
which looks exactly like a case we want to diagnose (very likely a
programming error).
So yes, it applies to the statement level and there only to control
statements.
Richard.
