Re: [PATCH] c++: Avoid some -Wreturn-type false positives with const{expr,eval} if [PR103991]

[Jakub Jelinek <jakub@redhat.com>]

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