Warning specifically for a returning noreturn

Warning specifically for a returning noreturn

[Julian Waters]

[-- Attachment #1: Type: text/plain, Size: 783 bytes --]

Hi all,

Currently to disable the warning that a noreturn method does return, it's
required to disable warnings entirely. This can be very inconvenient when
-Werror is enabled with a noreturn method that isn't specifically calling
something like std::abort() at the end, when one wants all other -Wall and
-Wextra warnings to be reported, for instance in the Java HotSpot VM (which
I'm currently adapting to compile with gcc on all supported platforms). Is
there a possibility we can add a disable warning option specifically for
this case? Something like -Wno-returning-noreturn. I'm interested in adding
this myself if it's not convenient for gcc's maintainers to do so at the
moment, but I'd need some guidance on where to look and what the relevant
code is

best regards,
Julian

Re: Warning specifically for a returning noreturn

[Andrew Pinski]

On Tue, Jul 4, 2023 at 5:54 PM Julian Waters via Gcc <gcc@gcc.gnu.org> wrote:
>
> Hi all,
>
> Currently to disable the warning that a noreturn method does return, it's
> required to disable warnings entirely. This can be very inconvenient when
> -Werror is enabled with a noreturn method that isn't specifically calling
> something like std::abort() at the end, when one wants all other -Wall and
> -Wextra warnings to be reported, for instance in the Java HotSpot VM (which
> I'm currently adapting to compile with gcc on all supported platforms). Is
> there a possibility we can add a disable warning option specifically for
> this case? Something like -Wno-returning-noreturn. I'm interested in adding
> this myself if it's not convenient for gcc's maintainers to do so at the
> moment, but I'd need some guidance on where to look and what the relevant
> code is

You could just add
__builtin_unreachable(); (or std::unreachable(); if you are C++23 or
unreachable() if you are using C23).
Or even add while(true) ;

I am pretty sure not having an option is on purpose and not really
interested in adding an option here because of the above workarounds.

Thanks,
Andrew Pinski

>
> best regards,
> Julian

Re: Warning specifically for a returning noreturn

[Julian Waters]

[-- Attachment #1: Type: text/plain, Size: 1727 bytes --]

Hi Andrew, thanks for the quick response,

What if the method has a return value? I know it sounds counterintuitive,
but in some places HotSpot relies on the noreturn attribute being applied
to methods that do return a value in an unreachable code path. Does the
unreachable builtin cover that case too?

best regards.
Julian

On Wed, Jul 5, 2023 at 9:07 AM Andrew Pinski <pinskia@gmail.com> wrote:

> On Tue, Jul 4, 2023 at 5:54 PM Julian Waters via Gcc <gcc@gcc.gnu.org>
> wrote:
> >
> > Hi all,
> >
> > Currently to disable the warning that a noreturn method does return, it's
> > required to disable warnings entirely. This can be very inconvenient when
> > -Werror is enabled with a noreturn method that isn't specifically calling
> > something like std::abort() at the end, when one wants all other -Wall
> and
> > -Wextra warnings to be reported, for instance in the Java HotSpot VM
> (which
> > I'm currently adapting to compile with gcc on all supported platforms).
> Is
> > there a possibility we can add a disable warning option specifically for
> > this case? Something like -Wno-returning-noreturn. I'm interested in
> adding
> > this myself if it's not convenient for gcc's maintainers to do so at the
> > moment, but I'd need some guidance on where to look and what the relevant
> > code is
>
> You could just add
> __builtin_unreachable(); (or std::unreachable(); if you are C++23 or
> unreachable() if you are using C23).
> Or even add while(true) ;
>
> I am pretty sure not having an option is on purpose and not really
> interested in adding an option here because of the above workarounds.
>
> Thanks,
> Andrew Pinski
>
> >
> > best regards,
> > Julian
>

Re: Warning specifically for a returning noreturn

[Andrew Pinski]

On Tue, Jul 4, 2023 at 6:32 PM Julian Waters <tanksherman27@gmail.com> wrote:
>
> Hi Andrew, thanks for the quick response,
>
> What if the method has a return value? I know it sounds counterintuitive, but in some places HotSpot relies on the noreturn attribute being applied to methods that do return a value in an unreachable code path. Does the unreachable builtin cover that case too?

It is wrong to use noreturn on a function other than one which has a
return type of void as documented.
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
:
```
It does not make sense for a noreturn function to have a return type
other than void.
```

Thanks,
Andrew Pinski


>
> best regards.
> Julian
>
> On Wed, Jul 5, 2023 at 9:07 AM Andrew Pinski <pinskia@gmail.com> wrote:
>>
>> On Tue, Jul 4, 2023 at 5:54 PM Julian Waters via Gcc <gcc@gcc.gnu.org> wrote:
>> >
>> > Hi all,
>> >
>> > Currently to disable the warning that a noreturn method does return, it's
>> > required to disable warnings entirely. This can be very inconvenient when
>> > -Werror is enabled with a noreturn method that isn't specifically calling
>> > something like std::abort() at the end, when one wants all other -Wall and
>> > -Wextra warnings to be reported, for instance in the Java HotSpot VM (which
>> > I'm currently adapting to compile with gcc on all supported platforms). Is
>> > there a possibility we can add a disable warning option specifically for
>> > this case? Something like -Wno-returning-noreturn. I'm interested in adding
>> > this myself if it's not convenient for gcc's maintainers to do so at the
>> > moment, but I'd need some guidance on where to look and what the relevant
>> > code is
>>
>> You could just add
>> __builtin_unreachable(); (or std::unreachable(); if you are C++23 or
>> unreachable() if you are using C23).
>> Or even add while(true) ;
>>
>> I am pretty sure not having an option is on purpose and not really
>> interested in adding an option here because of the above workarounds.
>>
>> Thanks,
>> Andrew Pinski
>>
>> >
>> > best regards,
>> > Julian

Re: Warning specifically for a returning noreturn

[LIU Hao]

[-- Attachment #1.1: Type: text/plain, Size: 802 bytes --]

在 2023/7/5 09:40, Andrew Pinski via Gcc 写道:
> It is wrong to use noreturn on a function other than one which has a
> return type of void as documented.
> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
> :
> ```
> It does not make sense for a noreturn function to have a return type
> other than void.
> ```

It makes sense for a callback or virtual function which requires a non-void return type, e.g.

```
class my_stream : public std::streambuf
   {
   protected:
     virtual
     int
     underflow() override;
   };

__attribute__((__noreturn__))   // [[noreturn]] won't work
int
my_stream::
underflow()
   {
     throw std::invalid_argument("not implemented");
   }
```



-- 
Best regards,
LIU Hao


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: Warning specifically for a returning noreturn

[Julian Waters]

[-- Attachment #1: Type: text/plain, Size: 2567 bytes --]

I see, thanks Andrew.

Anyone else have opinions on this besides Liu or Andrew? The responses have
been surprisingly quiet thus far

best regards,
Julian

On Wed, 5 Jul 2023, 09:40 Andrew Pinski, <pinskia@gmail.com> wrote:

> On Tue, Jul 4, 2023 at 6:32 PM Julian Waters <tanksherman27@gmail.com>
> wrote:
> >
> > Hi Andrew, thanks for the quick response,
> >
> > What if the method has a return value? I know it sounds
> counterintuitive, but in some places HotSpot relies on the noreturn
> attribute being applied to methods that do return a value in an unreachable
> code path. Does the unreachable builtin cover that case too?
>
> It is wrong to use noreturn on a function other than one which has a
> return type of void as documented.
>
> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
> :
> ```
> It does not make sense for a noreturn function to have a return type
> other than void.
> ```
>
> Thanks,
> Andrew Pinski
>
>
> >
> > best regards.
> > Julian
> >
> > On Wed, Jul 5, 2023 at 9:07 AM Andrew Pinski <pinskia@gmail.com> wrote:
> >>
> >> On Tue, Jul 4, 2023 at 5:54 PM Julian Waters via Gcc <gcc@gcc.gnu.org>
> wrote:
> >> >
> >> > Hi all,
> >> >
> >> > Currently to disable the warning that a noreturn method does return,
> it's
> >> > required to disable warnings entirely. This can be very inconvenient
> when
> >> > -Werror is enabled with a noreturn method that isn't specifically
> calling
> >> > something like std::abort() at the end, when one wants all other
> -Wall and
> >> > -Wextra warnings to be reported, for instance in the Java HotSpot VM
> (which
> >> > I'm currently adapting to compile with gcc on all supported
> platforms). Is
> >> > there a possibility we can add a disable warning option specifically
> for
> >> > this case? Something like -Wno-returning-noreturn. I'm interested in
> adding
> >> > this myself if it's not convenient for gcc's maintainers to do so at
> the
> >> > moment, but I'd need some guidance on where to look and what the
> relevant
> >> > code is
> >>
> >> You could just add
> >> __builtin_unreachable(); (or std::unreachable(); if you are C++23 or
> >> unreachable() if you are using C23).
> >> Or even add while(true) ;
> >>
> >> I am pretty sure not having an option is on purpose and not really
> >> interested in adding an option here because of the above workarounds.
> >>
> >> Thanks,
> >> Andrew Pinski
> >>
> >> >
> >> > best regards,
> >> > Julian
>

Re: Warning specifically for a returning noreturn

[Jonathan Wakely]

On Wed, 5 Jul 2023 at 12:01, Julian Waters via Gcc <gcc@gcc.gnu.org> wrote:
>
> I see, thanks Andrew.
>
> Anyone else have opinions on this besides Liu or Andrew? The responses have
> been surprisingly quiet thus far

IMHO all warnings should have an option controlling them, so that you
can disable them via pragmas.

But I agree that you shouldn't need to return from a noreturn
function, it can either throw or use __builtin_unreachable() on the
line where you currently return.


>
> best regards,
> Julian
>
> On Wed, 5 Jul 2023, 09:40 Andrew Pinski, <pinskia@gmail.com> wrote:
>
> > On Tue, Jul 4, 2023 at 6:32 PM Julian Waters <tanksherman27@gmail.com>
> > wrote:
> > >
> > > Hi Andrew, thanks for the quick response,
> > >
> > > What if the method has a return value? I know it sounds
> > counterintuitive, but in some places HotSpot relies on the noreturn
> > attribute being applied to methods that do return a value in an unreachable
> > code path. Does the unreachable builtin cover that case too?
> >
> > It is wrong to use noreturn on a function other than one which has a
> > return type of void as documented.
> >
> > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
> > :
> > ```
> > It does not make sense for a noreturn function to have a return type
> > other than void.
> > ```
> >
> > Thanks,
> > Andrew Pinski
> >
> >
> > >
> > > best regards.
> > > Julian
> > >
> > > On Wed, Jul 5, 2023 at 9:07 AM Andrew Pinski <pinskia@gmail.com> wrote:
> > >>
> > >> On Tue, Jul 4, 2023 at 5:54 PM Julian Waters via Gcc <gcc@gcc.gnu.org>
> > wrote:
> > >> >
> > >> > Hi all,
> > >> >
> > >> > Currently to disable the warning that a noreturn method does return,
> > it's
> > >> > required to disable warnings entirely. This can be very inconvenient
> > when
> > >> > -Werror is enabled with a noreturn method that isn't specifically
> > calling
> > >> > something like std::abort() at the end, when one wants all other
> > -Wall and
> > >> > -Wextra warnings to be reported, for instance in the Java HotSpot VM
> > (which
> > >> > I'm currently adapting to compile with gcc on all supported
> > platforms). Is
> > >> > there a possibility we can add a disable warning option specifically
> > for
> > >> > this case? Something like -Wno-returning-noreturn. I'm interested in
> > adding
> > >> > this myself if it's not convenient for gcc's maintainers to do so at
> > the
> > >> > moment, but I'd need some guidance on where to look and what the
> > relevant
> > >> > code is
> > >>
> > >> You could just add
> > >> __builtin_unreachable(); (or std::unreachable(); if you are C++23 or
> > >> unreachable() if you are using C23).
> > >> Or even add while(true) ;
> > >>
> > >> I am pretty sure not having an option is on purpose and not really
> > >> interested in adding an option here because of the above workarounds.
> > >>
> > >> Thanks,
> > >> Andrew Pinski
> > >>
> > >> >
> > >> > best regards,
> > >> > Julian
> >

Re: Warning specifically for a returning noreturn

[Julian Waters]

[-- Attachment #1: Type: text/plain, Size: 3705 bytes --]

Hi Jonathan,

Thanks for the reply, is there a place in gcc's source code I could look at
for this? As for the returning an explicit value from noreturn, I'm
unfortunately not the one who wrote the code that way; I'm merely a build
systems developer trying to get it to work with gcc :/

best regards,
Julian

On Wed, 5 Jul 2023, 19:26 Jonathan Wakely, <jwakely.gcc@gmail.com> wrote:

> On Wed, 5 Jul 2023 at 12:01, Julian Waters via Gcc <gcc@gcc.gnu.org>
> wrote:
> >
> > I see, thanks Andrew.
> >
> > Anyone else have opinions on this besides Liu or Andrew? The responses
> have
> > been surprisingly quiet thus far
>
> IMHO all warnings should have an option controlling them, so that you
> can disable them via pragmas.
>
> But I agree that you shouldn't need to return from a noreturn
> function, it can either throw or use __builtin_unreachable() on the
> line where you currently return.
>
>
> >
> > best regards,
> > Julian
> >
> > On Wed, 5 Jul 2023, 09:40 Andrew Pinski, <pinskia@gmail.com> wrote:
> >
> > > On Tue, Jul 4, 2023 at 6:32 PM Julian Waters <tanksherman27@gmail.com>
> > > wrote:
> > > >
> > > > Hi Andrew, thanks for the quick response,
> > > >
> > > > What if the method has a return value? I know it sounds
> > > counterintuitive, but in some places HotSpot relies on the noreturn
> > > attribute being applied to methods that do return a value in an
> unreachable
> > > code path. Does the unreachable builtin cover that case too?
> > >
> > > It is wrong to use noreturn on a function other than one which has a
> > > return type of void as documented.
> > >
> > >
> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
> > > :
> > > ```
> > > It does not make sense for a noreturn function to have a return type
> > > other than void.
> > > ```
> > >
> > > Thanks,
> > > Andrew Pinski
> > >
> > >
> > > >
> > > > best regards.
> > > > Julian
> > > >
> > > > On Wed, Jul 5, 2023 at 9:07 AM Andrew Pinski <pinskia@gmail.com>
> wrote:
> > > >>
> > > >> On Tue, Jul 4, 2023 at 5:54 PM Julian Waters via Gcc <
> gcc@gcc.gnu.org>
> > > wrote:
> > > >> >
> > > >> > Hi all,
> > > >> >
> > > >> > Currently to disable the warning that a noreturn method does
> return,
> > > it's
> > > >> > required to disable warnings entirely. This can be very
> inconvenient
> > > when
> > > >> > -Werror is enabled with a noreturn method that isn't specifically
> > > calling
> > > >> > something like std::abort() at the end, when one wants all other
> > > -Wall and
> > > >> > -Wextra warnings to be reported, for instance in the Java HotSpot
> VM
> > > (which
> > > >> > I'm currently adapting to compile with gcc on all supported
> > > platforms). Is
> > > >> > there a possibility we can add a disable warning option
> specifically
> > > for
> > > >> > this case? Something like -Wno-returning-noreturn. I'm interested
> in
> > > adding
> > > >> > this myself if it's not convenient for gcc's maintainers to do so
> at
> > > the
> > > >> > moment, but I'd need some guidance on where to look and what the
> > > relevant
> > > >> > code is
> > > >>
> > > >> You could just add
> > > >> __builtin_unreachable(); (or std::unreachable(); if you are C++23 or
> > > >> unreachable() if you are using C23).
> > > >> Or even add while(true) ;
> > > >>
> > > >> I am pretty sure not having an option is on purpose and not really
> > > >> interested in adding an option here because of the above
> workarounds.
> > > >>
> > > >> Thanks,
> > > >> Andrew Pinski
> > > >>
> > > >> >
> > > >> > best regards,
> > > >> > Julian
> > >
>

Re: Warning specifically for a returning noreturn

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 4622 bytes --]

On Wed, 5 Jul 2023, 14:13 Julian Waters, <tanksherman27@gmail.com> wrote:

> Hi Jonathan,
>
> Thanks for the reply, is there a place in gcc's source code I could look
> at for this?
>

This is a commit where I added four new warnings, and then changed the code
that issues those warnings to use the new OPT_Wxxx variables:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=ee336ecb2a7161bc28f6c5343d97870a8d15e177
You would want you do something similar. Define a warning in c.opt, then
find where that warning is printed and change the 0 to
OPT_Wreturn_in_noreturn (or whatever you call the new flag).

More generally, new options go in gcc/c-fsmily/c.opt so there are several
examples of adding new warnings in the history for that file:
https://gcc.gnu.org/git/?p=gcc.git;a=history;f=gcc/c-family/c.opt;h=4abdc8d0e77c6bded20829e35ac550c3a1b994dd;hb=refs/heads/master


As for the returning an explicit value from noreturn, I'm unfortunately not
> the one who wrote the code that way; I'm merely a build systems developer
> trying to get it to work with gcc :/
>
> best regards,
> Julian
>
> On Wed, 5 Jul 2023, 19:26 Jonathan Wakely, <jwakely.gcc@gmail.com> wrote:
>
>> On Wed, 5 Jul 2023 at 12:01, Julian Waters via Gcc <gcc@gcc.gnu.org>
>> wrote:
>> >
>> > I see, thanks Andrew.
>> >
>> > Anyone else have opinions on this besides Liu or Andrew? The responses
>> have
>> > been surprisingly quiet thus far
>>
>> IMHO all warnings should have an option controlling them, so that you
>> can disable them via pragmas.
>>
>> But I agree that you shouldn't need to return from a noreturn
>> function, it can either throw or use __builtin_unreachable() on the
>> line where you currently return.
>>
>>
>> >
>> > best regards,
>> > Julian
>> >
>> > On Wed, 5 Jul 2023, 09:40 Andrew Pinski, <pinskia@gmail.com> wrote:
>> >
>> > > On Tue, Jul 4, 2023 at 6:32 PM Julian Waters <tanksherman27@gmail.com
>> >
>> > > wrote:
>> > > >
>> > > > Hi Andrew, thanks for the quick response,
>> > > >
>> > > > What if the method has a return value? I know it sounds
>> > > counterintuitive, but in some places HotSpot relies on the noreturn
>> > > attribute being applied to methods that do return a value in an
>> unreachable
>> > > code path. Does the unreachable builtin cover that case too?
>> > >
>> > > It is wrong to use noreturn on a function other than one which has a
>> > > return type of void as documented.
>> > >
>> > >
>> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
>> > > :
>> > > ```
>> > > It does not make sense for a noreturn function to have a return type
>> > > other than void.
>> > > ```
>> > >
>> > > Thanks,
>> > > Andrew Pinski
>> > >
>> > >
>> > > >
>> > > > best regards.
>> > > > Julian
>> > > >
>> > > > On Wed, Jul 5, 2023 at 9:07 AM Andrew Pinski <pinskia@gmail.com>
>> wrote:
>> > > >>
>> > > >> On Tue, Jul 4, 2023 at 5:54 PM Julian Waters via Gcc <
>> gcc@gcc.gnu.org>
>> > > wrote:
>> > > >> >
>> > > >> > Hi all,
>> > > >> >
>> > > >> > Currently to disable the warning that a noreturn method does
>> return,
>> > > it's
>> > > >> > required to disable warnings entirely. This can be very
>> inconvenient
>> > > when
>> > > >> > -Werror is enabled with a noreturn method that isn't specifically
>> > > calling
>> > > >> > something like std::abort() at the end, when one wants all other
>> > > -Wall and
>> > > >> > -Wextra warnings to be reported, for instance in the Java
>> HotSpot VM
>> > > (which
>> > > >> > I'm currently adapting to compile with gcc on all supported
>> > > platforms). Is
>> > > >> > there a possibility we can add a disable warning option
>> specifically
>> > > for
>> > > >> > this case? Something like -Wno-returning-noreturn. I'm
>> interested in
>> > > adding
>> > > >> > this myself if it's not convenient for gcc's maintainers to do
>> so at
>> > > the
>> > > >> > moment, but I'd need some guidance on where to look and what the
>> > > relevant
>> > > >> > code is
>> > > >>
>> > > >> You could just add
>> > > >> __builtin_unreachable(); (or std::unreachable(); if you are C++23
>> or
>> > > >> unreachable() if you are using C23).
>> > > >> Or even add while(true) ;
>> > > >>
>> > > >> I am pretty sure not having an option is on purpose and not really
>> > > >> interested in adding an option here because of the above
>> workarounds.
>> > > >>
>> > > >> Thanks,
>> > > >> Andrew Pinski
>> > > >>
>> > > >> >
>> > > >> > best regards,
>> > > >> > Julian
>> > >
>>
>

Re: Warning specifically for a returning noreturn

[Julian Waters]

[-- Attachment #1: Type: text/plain, Size: 4876 bytes --]

Hi all,

I've found the places responsible for the warnings, but before I do
anything I'd like to discuss a couple of things.

1. What would a good name for the warning switch be? How does
-Wreturning-noreturn sound?
2. I've thought about this for a while, and I feel like throwing a warning
for a noreturn method that isn't explicitly noreturn in the Control Flow
Graph is a little too harsh. The point of the attribute is to hint to gcc
that the method will never return even if it appears so, and requiring that
the body explicitly do something like call abort() or loop infinitely kind
of defeats the purpose of the attribute, in my opinion
3. If (2) is just me missing something, should I split the warning into 2
different warnings for a noreturn definition with an explicit return
statement and an implicit one in the case of a method not explicitly
throwing/looping infinitely, etc?

Thoughts?

best regards,
Julian

On Wed, Jul 5, 2023 at 9:13 PM Julian Waters <tanksherman27@gmail.com>
wrote:

> Hi Jonathan,
>
> Thanks for the reply, is there a place in gcc's source code I could look
> at for this? As for the returning an explicit value from noreturn, I'm
> unfortunately not the one who wrote the code that way; I'm merely a build
> systems developer trying to get it to work with gcc :/
>
> best regards,
> Julian
>
> On Wed, 5 Jul 2023, 19:26 Jonathan Wakely, <jwakely.gcc@gmail.com> wrote:
>
>> On Wed, 5 Jul 2023 at 12:01, Julian Waters via Gcc <gcc@gcc.gnu.org>
>> wrote:
>> >
>> > I see, thanks Andrew.
>> >
>> > Anyone else have opinions on this besides Liu or Andrew? The responses
>> have
>> > been surprisingly quiet thus far
>>
>> IMHO all warnings should have an option controlling them, so that you
>> can disable them via pragmas.
>>
>> But I agree that you shouldn't need to return from a noreturn
>> function, it can either throw or use __builtin_unreachable() on the
>> line where you currently return.
>>
>>
>> >
>> > best regards,
>> > Julian
>> >
>> > On Wed, 5 Jul 2023, 09:40 Andrew Pinski, <pinskia@gmail.com> wrote:
>> >
>> > > On Tue, Jul 4, 2023 at 6:32 PM Julian Waters <tanksherman27@gmail.com
>> >
>> > > wrote:
>> > > >
>> > > > Hi Andrew, thanks for the quick response,
>> > > >
>> > > > What if the method has a return value? I know it sounds
>> > > counterintuitive, but in some places HotSpot relies on the noreturn
>> > > attribute being applied to methods that do return a value in an
>> unreachable
>> > > code path. Does the unreachable builtin cover that case too?
>> > >
>> > > It is wrong to use noreturn on a function other than one which has a
>> > > return type of void as documented.
>> > >
>> > >
>> https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
>> > > :
>> > > ```
>> > > It does not make sense for a noreturn function to have a return type
>> > > other than void.
>> > > ```
>> > >
>> > > Thanks,
>> > > Andrew Pinski
>> > >
>> > >
>> > > >
>> > > > best regards.
>> > > > Julian
>> > > >
>> > > > On Wed, Jul 5, 2023 at 9:07 AM Andrew Pinski <pinskia@gmail.com>
>> wrote:
>> > > >>
>> > > >> On Tue, Jul 4, 2023 at 5:54 PM Julian Waters via Gcc <
>> gcc@gcc.gnu.org>
>> > > wrote:
>> > > >> >
>> > > >> > Hi all,
>> > > >> >
>> > > >> > Currently to disable the warning that a noreturn method does
>> return,
>> > > it's
>> > > >> > required to disable warnings entirely. This can be very
>> inconvenient
>> > > when
>> > > >> > -Werror is enabled with a noreturn method that isn't specifically
>> > > calling
>> > > >> > something like std::abort() at the end, when one wants all other
>> > > -Wall and
>> > > >> > -Wextra warnings to be reported, for instance in the Java
>> HotSpot VM
>> > > (which
>> > > >> > I'm currently adapting to compile with gcc on all supported
>> > > platforms). Is
>> > > >> > there a possibility we can add a disable warning option
>> specifically
>> > > for
>> > > >> > this case? Something like -Wno-returning-noreturn. I'm
>> interested in
>> > > adding
>> > > >> > this myself if it's not convenient for gcc's maintainers to do
>> so at
>> > > the
>> > > >> > moment, but I'd need some guidance on where to look and what the
>> > > relevant
>> > > >> > code is
>> > > >>
>> > > >> You could just add
>> > > >> __builtin_unreachable(); (or std::unreachable(); if you are C++23
>> or
>> > > >> unreachable() if you are using C23).
>> > > >> Or even add while(true) ;
>> > > >>
>> > > >> I am pretty sure not having an option is on purpose and not really
>> > > >> interested in adding an option here because of the above
>> workarounds.
>> > > >>
>> > > >> Thanks,
>> > > >> Andrew Pinski
>> > > >>
>> > > >> >
>> > > >> > best regards,
>> > > >> > Julian
>> > >
>>
>

Re: Warning specifically for a returning noreturn

[Jonathan Wakely]

On Fri, 21 Jul 2023 at 04:28, Julian Waters via Gcc <gcc@gcc.gnu.org> wrote:
>
> Hi all,
>
> I've found the places responsible for the warnings, but before I do
> anything I'd like to discuss a couple of things.
>
> 1. What would a good name for the warning switch be? How does
> -Wreturning-noreturn sound?
> 2. I've thought about this for a while, and I feel like throwing a warning
> for a noreturn method that isn't explicitly noreturn in the Control Flow
> Graph is a little too harsh. The point of the attribute is to hint to gcc
> that the method will never return even if it appears so,

Is it? My understanding is that it's for functions the compiler can't
see the body of, e.g. user-defined functions similar to abort(). A
function that doesn't return "even if it appears so" implies the
function body is visible to the compiler. I don't think that's the
primary use case. The compiler is already perfectly capable of
optimizing callers appropriately if the function clearly never
returns, you don't need the attribute for those cases.

> and requiring that
> the body explicitly do something like call abort() or loop infinitely kind
> of defeats the purpose of the attribute, in my opinion

I disagree, because the attribute is for the benefit of the calling
code, not the function body itself. And so it still serves that
purpose whatever the body of the function looks like.

If you've added the attribute, so that calling code can be optimized
accordingly, but then the body actually does return ... what are you
playing it? That's weird and certainly deserves a warning. The calling
code might not work correctly if the function does return, because
it's been optimized assumed that can never happen, so you cause the
program to have undefined behaviour by returning from a noreturn
function. That certainly deserves a warning.

The attribute's primary purpose is to inform callers the function
won't return. A side effect of that is that the function itself might
generate warnings if it appears to violate the contract you've made
(that it won't return). To avoid those warnings, you need to write the
function body so that it doesn't return. So instead of allowing the
function to return, call another noreturn function (like abort), or
add a __builtin_unreachable (or call std::unreachable in C++23), or
throw an exception.

> 3.
 If (2) is just me missing something, should I split the warning into 2
> different warnings for a noreturn definition with an explicit return
> statement and an implicit one in the case of a method not explicitly
> throwing/looping infinitely, etc?

I'm not sure it's worth it, as they're closely related. Although I
suppose you could have a noreturn function that must have a non-void
return type in order to conform to some expected API (e.g. a virtual
function, or a function who's address is taken and used as a
callback), so want to disable the warning about a non-void return, but
still get warnings for the function body to help you ensure it really
doesn't return by mistake.

Re: Warning specifically for a returning noreturn

[Julian Waters]

[-- Attachment #1: Type: text/plain, Size: 3433 bytes --]

I see. I think it may be appropriate to adopt what clang has done and have
one -Winvalid-noreturn for both cases, thoughts?

On Fri, Jul 21, 2023 at 5:43 PM Jonathan Wakely <jwakely.gcc@gmail.com>
wrote:

> On Fri, 21 Jul 2023 at 04:28, Julian Waters via Gcc <gcc@gcc.gnu.org>
> wrote:
> >
> > Hi all,
> >
> > I've found the places responsible for the warnings, but before I do
> > anything I'd like to discuss a couple of things.
> >
> > 1. What would a good name for the warning switch be? How does
> > -Wreturning-noreturn sound?
> > 2. I've thought about this for a while, and I feel like throwing a
> warning
> > for a noreturn method that isn't explicitly noreturn in the Control Flow
> > Graph is a little too harsh. The point of the attribute is to hint to gcc
> > that the method will never return even if it appears so,
>
> Is it? My understanding is that it's for functions the compiler can't
> see the body of, e.g. user-defined functions similar to abort(). A
> function that doesn't return "even if it appears so" implies the
> function body is visible to the compiler. I don't think that's the
> primary use case. The compiler is already perfectly capable of
> optimizing callers appropriately if the function clearly never
> returns, you don't need the attribute for those cases.
>
> > and requiring that
> > the body explicitly do something like call abort() or loop infinitely
> kind
> > of defeats the purpose of the attribute, in my opinion
>
> I disagree, because the attribute is for the benefit of the calling
> code, not the function body itself. And so it still serves that
> purpose whatever the body of the function looks like.
>
> If you've added the attribute, so that calling code can be optimized
> accordingly, but then the body actually does return ... what are you
> playing it? That's weird and certainly deserves a warning. The calling
> code might not work correctly if the function does return, because
> it's been optimized assumed that can never happen, so you cause the
> program to have undefined behaviour by returning from a noreturn
> function. That certainly deserves a warning.
>
> The attribute's primary purpose is to inform callers the function
> won't return. A side effect of that is that the function itself might
> generate warnings if it appears to violate the contract you've made
> (that it won't return). To avoid those warnings, you need to write the
> function body so that it doesn't return. So instead of allowing the
> function to return, call another noreturn function (like abort), or
> add a __builtin_unreachable (or call std::unreachable in C++23), or
> throw an exception.
>
> > 3.
>  If (2) is just me missing something, should I split the warning into 2
> > different warnings for a noreturn definition with an explicit return
> > statement and an implicit one in the case of a method not explicitly
> > throwing/looping infinitely, etc?
>
> I'm not sure it's worth it, as they're closely related. Although I
> suppose you could have a noreturn function that must have a non-void
> return type in order to conform to some expected API (e.g. a virtual
> function, or a function who's address is taken and used as a
> callback), so want to disable the warning about a non-void return, but
> still get warnings for the function body to help you ensure it really
> doesn't return by mistake.
>

Re: Warning specifically for a returning noreturn

[Eric Gallager]

On Wed, Jul 5, 2023 at 7:28 AM Jonathan Wakely via Gcc <gcc@gcc.gnu.org> wrote:
>
> On Wed, 5 Jul 2023 at 12:01, Julian Waters via Gcc <gcc@gcc.gnu.org> wrote:
> >
> > I see, thanks Andrew.
> >
> > Anyone else have opinions on this besides Liu or Andrew? The responses have
> > been surprisingly quiet thus far
>
> IMHO all warnings should have an option controlling them, so that you
> can disable them via pragmas.
>

This is bug 44209, for reference:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44209

> But I agree that you shouldn't need to return from a noreturn
> function, it can either throw or use __builtin_unreachable() on the
> line where you currently return.
>
>
> >
> > best regards,
> > Julian
> >
> > On Wed, 5 Jul 2023, 09:40 Andrew Pinski, <pinskia@gmail.com> wrote:
> >
> > > On Tue, Jul 4, 2023 at 6:32 PM Julian Waters <tanksherman27@gmail.com>
> > > wrote:
> > > >
> > > > Hi Andrew, thanks for the quick response,
> > > >
> > > > What if the method has a return value? I know it sounds
> > > counterintuitive, but in some places HotSpot relies on the noreturn
> > > attribute being applied to methods that do return a value in an unreachable
> > > code path. Does the unreachable builtin cover that case too?
> > >
> > > It is wrong to use noreturn on a function other than one which has a
> > > return type of void as documented.
> > >
> > > https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
> > > :
> > > ```
> > > It does not make sense for a noreturn function to have a return type
> > > other than void.
> > > ```
> > >
> > > Thanks,
> > > Andrew Pinski
> > >
> > >
> > > >
> > > > best regards.
> > > > Julian
> > > >
> > > > On Wed, Jul 5, 2023 at 9:07 AM Andrew Pinski <pinskia@gmail.com> wrote:
> > > >>
> > > >> On Tue, Jul 4, 2023 at 5:54 PM Julian Waters via Gcc <gcc@gcc.gnu.org>
> > > wrote:
> > > >> >
> > > >> > Hi all,
> > > >> >
> > > >> > Currently to disable the warning that a noreturn method does return,
> > > it's
> > > >> > required to disable warnings entirely. This can be very inconvenient
> > > when
> > > >> > -Werror is enabled with a noreturn method that isn't specifically
> > > calling
> > > >> > something like std::abort() at the end, when one wants all other
> > > -Wall and
> > > >> > -Wextra warnings to be reported, for instance in the Java HotSpot VM
> > > (which
> > > >> > I'm currently adapting to compile with gcc on all supported
> > > platforms). Is
> > > >> > there a possibility we can add a disable warning option specifically
> > > for
> > > >> > this case? Something like -Wno-returning-noreturn. I'm interested in
> > > adding
> > > >> > this myself if it's not convenient for gcc's maintainers to do so at
> > > the
> > > >> > moment, but I'd need some guidance on where to look and what the
> > > relevant
> > > >> > code is
> > > >>
> > > >> You could just add
> > > >> __builtin_unreachable(); (or std::unreachable(); if you are C++23 or
> > > >> unreachable() if you are using C23).
> > > >> Or even add while(true) ;
> > > >>
> > > >> I am pretty sure not having an option is on purpose and not really
> > > >> interested in adding an option here because of the above workarounds.
> > > >>
> > > >> Thanks,
> > > >> Andrew Pinski
> > > >>
> > > >> >
> > > >> > best regards,
> > > >> > Julian
> > >