On Thu, Jan 13, 2022 at 04:09:22PM -0500, Jason Merrill wrote: > > The changes done to genericize_if_stmt in order to improve > > -Wunreachable-code* warning (which Richi didn't actually commit > > for GCC 12) are I think fine for normal ifs, but for constexpr if > > and consteval if we have two competing warnings. > > The problem is that we replace the non-taken clause (then or else) > > with void_node and keep the if (cond) { something } else {} > > or if (cond) {} else { something }; in the IL. > > This helps -Wunreachable-code*, if something can't fallthru but the > > non-taken clause can, we don't warn about code after it because it > > is still (in theory) reachable. > > But if the non-taken branch can't fallthru, we can get false positive > > -Wreturn-type warnings (which are enabled by default) if there is > > nothing after the if and the taken branch can't fallthru either. > > Perhaps we should replace the non-taken clause with __builtin_unreachable() > instead of void_node?
It depends. If the non-taken clause doesn't exist, is empty or otherwise can fallthru, then using void_node for it is what we want. If it exists and can't fallthru, then __builtin_unreachable() is one possibility, but for all purpose if (1) something else __builtin_unreachable(); is equivalent to genericization of it as something and if (0) __builtin_unreachable(); else something too. The main problem is what to do for the consteval if that throws away the non-taken clause too early, whether we can do block_may_fallthru already where we throw it away or not. If we can do that, we could as right now clear the non-taken clause if it can fallthru and otherwise either set some flag on the IF_STMT or set the non-taken clause to __builtin_unreachable or endless empty loop etc., ideally something as cheap as possible. > And/or block_may_fallthru could handle INTEGER_CST op0? That is what I'm doing for consteval if in the patch because the info whether the non-taken clause can fallthru is lost. We can't do that for normal if, because the non-taken clause could have labels in it to which something jumps. But, block_may_fallthru isn't actually what is used for the -Wreturn-type warning, I think we warn only at cfg creation. Jakub