On Thu, 25 Nov 2021, Richard Biener wrote: > On Wed, 24 Nov 2021, Michael Matz wrote: > > > Hello, > > > > On Wed, 24 Nov 2021, Richard Biener wrote: > > > > > >> +/* Unreachable code in if (0) block. */ > > > >> +void baz(int *p) > > > >> +{ > > > >> + if (0) > > > >> + { > > > >> + return; /* { dg-bogus "not reachable" } */ > > > > > > > >Hmm? Why are you explicitely saying that warning here would be bogus? > > > > > > Because I don't think we want to warn here. Such code is common from > > > template instantiation or macro expansion. > > > > Like all code with an (const-propagated) explicit 'if (0)', which is of > > course the reason why -Wunreachable-code is a challenge. > > OK, so I probably shouldn't have taken -Wunreachable-code but named > it somehow differently. We want to diagnose obvious programming > mistakes, not (source code) optimization opportunities. So > > int foo (int i) > { > return i; > i += 1; > return i; > } > > should be diagnosed for example but not so > > int foo (int i) > { > if (USE_NOOP_FOO) > return i; > i += 1; > return i; > } > > and compiling with -DUSE_NOOP_FOO=1 > > > IOW: I could > > accept your argument but then wonder why you want to warn about the second > > statement of the guarded block. The situation was: > > > > if (0) { > > return; // (1) don't warn here? > > whatever++; // (2) but warn here? > > because as said above, the whatever++ will never be reachable even if > you change the condition in the if(). See my response to Martin where > I said I think if (0) of a block is a good way to comment it out > but keep it syntactically correct. > > > } > > > > That seems even more confusing. So you don't want to warn about > > unreachable code (the 'return') but you do want to warn about unreachable > > code within unreachable code (point (2) is unreachable because of the > > if(0) and because of the return). If your worry is macro/template > > expansion resulting if(0)'s then I don't see why you would only disable > > warnings for some of the statements therein. > > The point is not to disable the warning for some statements therein > but to avoid diagnosing following stmts. > > > It seems we are actually interested in code unreachable via fallthrough or > > labels, not in all unreachable code, so maybe the warning is mis-named. > > Yes, that's definitely the case - I was too lazy to re-use the old > option name here. But I don't have a good name at hand, maybe clang > has an option covering the cases I'm thinking about. > > Btw, the diagnostic spotted qsort_chk doing > > if (CMP (i1, i2)) > break; > else if (CMP (i2, i1)) > return ERR2 (i1, i2); > > where ERR2 expands to a call to a noreturn void "returning" > qsort_chk_error, so the 'return' stmt is not reachable. Not exactly > a bug but somewhat difficult to avoid the diagnostic for. I suppose > the pointless 'return' is to make it more visible that the loop > terminates here (albeit we don't return normally). > > Likewise we diagnose (c_tree_equal): > > default: > gcc_unreachable (); > } > /* We can get here with --disable-checking. */ > return false; > > where the 'return false' is never reachable. The return was likely > inserted to avoid very strange error paths then the unreachable > falls through to some other random function.
It also finds this strange code in label_rtx_for_bb: /* Find the tree label if it is present. */ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) { glabel *lab_stmt; lab_stmt = dyn_cast <glabel *> (gsi_stmt (gsi)); if (!lab_stmt) break; lab = gimple_label_label (lab_stmt); if (DECL_NONLOCAL (lab)) break; return jump_target_rtx (lab); } diagnosing /home/rguenther/src/trunk/gcc/cfgexpand.c:2476:60: error: statement is not reachable [-Werror=unreachable-code] 2476 | for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi)) | ~~~~~~~~~^~~~~~ indeed the loop looks pointless. Unless the DECL_NONLOCAL case was meant to continue; Richard.