This is an archive of the discontinued LLVM Phabricator instance.

[libc] Add a convenience type spec for _Noreturn.
AbandonedPublic

Authored by sivachandra on Feb 28 2020, 4:13 PM.

Details

Reviewers
abrachet

Diff Detail

Event Timeline

sivachandra created this revision.Feb 28 2020, 4:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2020, 4:13 PM

This is helpful for D74949, but _Noreturn is not recognized by C++ compilers, a solution is to include <stdnoreturn.h> which will define noreturn only as _Noreturn for C. So using NoReturn would also require we make sure include <stdnoreturn.h> which is possible but perhaps not ideal.

Do we have any obligation to support prior to C11? If so then <stdnoreturn.h> would not be an option we would just have to make our own header, which is not difficult.

It's worth mentioning that a quick test on compiler explorer of MSVC suggests that they don't support _Noreturn or stdnoreturn.h even with -std=c11, but I don't think the burden should be on us to work around non conforming compilers.

Add a macro to define _Noreturn to [[noreturn]] for C++ compilations.

This is helpful for D74949, but _Noreturn is not recognized by C++ compilers, a solution is to include <stdnoreturn.h> which will define noreturn only as _Noreturn for C. So using NoReturn would also require we make sure include <stdnoreturn.h> which is possible but perhaps not ideal.

Indeed! I only tested with Clang before I sent this patch out and it worked so I assumed compilers just support it as their extension. After seeing your comment, I tried with GCC and it says it does not understand _Noreturn in C++ compilations. So, I have now added a macro to define _Noreturn to [[noreturn]] for C++ compilations. Feel free to absorb this change in to your change adding abort and _Exit.

Do we have any obligation to support prior to C11? If so then <stdnoreturn.h> would not be an option we would just have to make our own header, which is not difficult.

No. We do not want to support anything older than C17 (which is C11 + bug fixes.) But at the same time, we do not want to include another standard header also.

It's worth mentioning that a quick test on compiler explorer of MSVC suggests that they don't support _Noreturn or stdnoreturn.h even with -std=c11, but I don't think the burden should be on us to work around non conforming compilers.

Yes. I do not think we have to bend over backwards to support non-compliant compilers. In this case, it wouldn't really be as big as bending over backwards, but we do not want to set precedents.

abrachet accepted this revision.Feb 29 2020, 12:10 AM

Thanks, thats a way easier solution to put that definition in __llvm-libc-common.h!

Feel free to absorb this change in to your change adding abort and _Exit.

You can go ahead and commit this, whatever is easier for you. It would be at most one merge conflict for that patch so no big deal, either way works.

This revision is now accepted and ready to land.Feb 29 2020, 12:10 AM
sivachandra abandoned this revision.Mar 7 2020, 3:31 PM

This went in as part of another change.