* Warning specifically for a returning noreturn @ 2023-07-05 0:52 Julian Waters 2023-07-05 1:07 ` Andrew Pinski 0 siblings, 1 reply; 13+ messages in thread From: Julian Waters @ 2023-07-05 0:52 UTC (permalink / raw) To: gcc [-- 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 0:52 Warning specifically for a returning noreturn Julian Waters @ 2023-07-05 1:07 ` Andrew Pinski 2023-07-05 1:31 ` Julian Waters 0 siblings, 1 reply; 13+ messages in thread From: Andrew Pinski @ 2023-07-05 1:07 UTC (permalink / raw) To: Julian Waters; +Cc: gcc 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 1:07 ` Andrew Pinski @ 2023-07-05 1:31 ` Julian Waters 2023-07-05 1:40 ` Andrew Pinski 0 siblings, 1 reply; 13+ messages in thread From: Julian Waters @ 2023-07-05 1:31 UTC (permalink / raw) To: Andrew Pinski, gcc [-- 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 1:31 ` Julian Waters @ 2023-07-05 1:40 ` Andrew Pinski 2023-07-05 5:40 ` LIU Hao 2023-07-05 11:00 ` Julian Waters 0 siblings, 2 replies; 13+ messages in thread From: Andrew Pinski @ 2023-07-05 1:40 UTC (permalink / raw) To: Julian Waters; +Cc: gcc 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 1:40 ` Andrew Pinski @ 2023-07-05 5:40 ` LIU Hao 2023-07-05 11:00 ` Julian Waters 1 sibling, 0 replies; 13+ messages in thread From: LIU Hao @ 2023-07-05 5:40 UTC (permalink / raw) To: Andrew Pinski, Julian Waters; +Cc: gcc [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 1:40 ` Andrew Pinski 2023-07-05 5:40 ` LIU Hao @ 2023-07-05 11:00 ` Julian Waters 2023-07-05 11:26 ` Jonathan Wakely 1 sibling, 1 reply; 13+ messages in thread From: Julian Waters @ 2023-07-05 11:00 UTC (permalink / raw) To: Andrew Pinski, gcc [-- 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 11:00 ` Julian Waters @ 2023-07-05 11:26 ` Jonathan Wakely 2023-07-05 13:13 ` Julian Waters 2023-07-05 16:50 ` Eric Gallager 0 siblings, 2 replies; 13+ messages in thread From: Jonathan Wakely @ 2023-07-05 11:26 UTC (permalink / raw) To: Julian Waters; +Cc: Andrew Pinski, gcc 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 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 11:26 ` Jonathan Wakely @ 2023-07-05 13:13 ` Julian Waters 2023-07-05 13:20 ` Jonathan Wakely 2023-07-21 3:26 ` Julian Waters 2023-07-05 16:50 ` Eric Gallager 1 sibling, 2 replies; 13+ messages in thread From: Julian Waters @ 2023-07-05 13:13 UTC (permalink / raw) To: Jonathan Wakely, gcc [-- 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 > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 13:13 ` Julian Waters @ 2023-07-05 13:20 ` Jonathan Wakely 2023-07-21 3:26 ` Julian Waters 1 sibling, 0 replies; 13+ messages in thread From: Jonathan Wakely @ 2023-07-05 13:20 UTC (permalink / raw) To: Julian Waters; +Cc: gcc [-- 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 >> > > >> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 13:13 ` Julian Waters 2023-07-05 13:20 ` Jonathan Wakely @ 2023-07-21 3:26 ` Julian Waters 2023-07-21 9:43 ` Jonathan Wakely 1 sibling, 1 reply; 13+ messages in thread From: Julian Waters @ 2023-07-21 3:26 UTC (permalink / raw) To: gcc [-- 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 >> > > >> > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-21 3:26 ` Julian Waters @ 2023-07-21 9:43 ` Jonathan Wakely 2023-07-25 2:38 ` Julian Waters 0 siblings, 1 reply; 13+ messages in thread From: Jonathan Wakely @ 2023-07-21 9:43 UTC (permalink / raw) To: Julian Waters; +Cc: gcc 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-21 9:43 ` Jonathan Wakely @ 2023-07-25 2:38 ` Julian Waters 0 siblings, 0 replies; 13+ messages in thread From: Julian Waters @ 2023-07-25 2:38 UTC (permalink / raw) To: Jonathan Wakely, gcc [-- 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. > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Warning specifically for a returning noreturn 2023-07-05 11:26 ` Jonathan Wakely 2023-07-05 13:13 ` Julian Waters @ 2023-07-05 16:50 ` Eric Gallager 1 sibling, 0 replies; 13+ messages in thread From: Eric Gallager @ 2023-07-05 16:50 UTC (permalink / raw) To: Jonathan Wakely; +Cc: Julian Waters, Andrew Pinski, gcc 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 > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-07-25 2:39 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-05 0:52 Warning specifically for a returning noreturn Julian Waters 2023-07-05 1:07 ` Andrew Pinski 2023-07-05 1:31 ` Julian Waters 2023-07-05 1:40 ` Andrew Pinski 2023-07-05 5:40 ` LIU Hao 2023-07-05 11:00 ` Julian Waters 2023-07-05 11:26 ` Jonathan Wakely 2023-07-05 13:13 ` Julian Waters 2023-07-05 13:20 ` Jonathan Wakely 2023-07-21 3:26 ` Julian Waters 2023-07-21 9:43 ` Jonathan Wakely 2023-07-25 2:38 ` Julian Waters 2023-07-05 16:50 ` Eric Gallager
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).