On Thu, 25 Nov 2021, Richard Biener wrote: > 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.
So another case in GCC is if (WORDS_BIG_ENDIAN == BYTES_BIG_ENDIAN) ... else { /* Assert that we're only dealing with the PDP11 case. */ gcc_assert (!BYTES_BIG_ENDIAN); gcc_assert (WORDS_BIG_ENDIAN); cpp_define (pfile, "__BYTE_ORDER__=__ORDER_PDP_ENDIAN__"); where that macro expands to ((void)(!(!0) ? fancy_abort ("/home/rguenther/src/trunk/gcc/cppbuiltin.c", 180, __FUNCTION__), 0 : 0)); ((void)(!(0) ? fancy_abort ("/home/rguenther/src/trunk/gcc/cppbuiltin.c", 181, __FUNCTION__), 0 : 0)); cpp_define (pfile, "__BYTE_ORDER__=__ORDER_PDP_ENDIAN__"); and the frontend elides the COND_EXPRs making the cpp_define unreachable. That's only exposed because we no longer elide the if (1) guardingt this else path ... Also this is a case where we definitely do not want to diagnose that either the else or the true path is statically unreachable. Richard.