Details
- Reviewers
abrachet
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
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.
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.
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.