On 1/13/22 16:23, Jakub Jelinek wrote:
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.
Fair enough. The patch is OK.
Jason
