Re: [RFC PATCH v1] perf build: Fix build for clang's -Wunreachable-code

[David Laight <david.laight.linux@gmail.com>]

On Fri, 30 May 2025 15:33:33 -0700
Namhyung Kim <namhyung@kernel.org> wrote:

...
> > > No, I meant we may not need the __builtin_unreachable() at the callsites.
> > >
> > > Would it complain this code?
> > >
> > >   if (some_bad_option_use)
> > >     usage_with_options(...);
> > >
> > >   /* normal code path */  
> > 
> > Right that would fix -Wunreachable, but the existing code would be:
> > 
> >   if (some_bad_option_use) {
> >     usage_with_options(...);
> >     exit(..);
> >   }
> >   /* normal code path */
> > 
> > Letting you know that "normal code path" couldn't be fallen into after
> > usage_with_options(...). To make the behavior more obvious we could
> > rename usage_with_options():
> > 
> >   if (some_bad_option_use)
> >     usage_with_options_and_exit(...);
> > 
> >   /* normal code path */
> > 
> > What I've done is:
> > 
> >   if (some_bad_option_use) {
> >     usage_with_options(...);
> >     __builtin_unreachable();
> >   }
> >   /* normal code path */

Isn't that just wrong?
Doesn't __builtin_unreachable() tell the compiler the code can't be executed.
In this case you want the opposite - something that the compiler will
whinge about if it finds a path that does execute the code.

In terms of source code readability it is a also horrid.
It is pretty common for an application to have a lot of 'usage' errors
and you want them to take as little vertical code space as possible.
That is the whole reason why it is common for a usage() function to
call exit() rather than return.

	David

> > 
> > My reasoning is that usage_with_options() doesn't obviously on the
> > face of it call exit and never return. To make that clear we could add
> > a comment:
> > 
> >   if (some_bad_option_use) {
> >     usage_with_options(...);
> >     /* usage_with_options never returns as it always calls exit */
> >   }
> >   /* normal code path */
> > 
> > But my preference is to use __builtin_unreachable as that is the same
> > as the comment but is also something the compiler can trap on were it
> > not true.  
> 
> I see, thanks for the explanation.  That part looks ok then.
> 
> Thanks,
> Namhyung
> 
>