[libcxx-commits] [PATCH] D119152: [libc++] Implement P0627R6 (Function to mark unreachable code)

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added a comment.

In D119152#3309138 <https://reviews.llvm.org/D119152#3309138>, @ldionne wrote:

> In D119152#3306276 <https://reviews.llvm.org/D119152#3306276>, @philnik wrote:
>
>> How do you expect the libc++-specific test to look? The compiler won't ignore that there is a `__builtin_unreachable` in the way and just remove all the code after that. We don't control the behavior in this case. We don't even have to do anything in `unreachable()`. It can just be empty and marked with `[[noreturn]]` and it would work pretty much the same.  https://godbolt.org/z/GvbnG76nW
>
> Sorry, `.fail.cpp` wasn't accurate. In `<test>.pass.cpp`:
>
>   // XFAIL: *
>   
>   int main(int, char**) {
>     std::unreachable();
>   
>     return 0;
>   }
>
> When you run this, it will do `zsh: illegal hardware instruction  ./a.out` if `__builtin_unreachable()` is used, and it will call `std::abort()` otherwise. In both cases, the executable will return with non-zero. And since the test is `XFAIL`ed, it will expect that non-zero and be reported as an expected failure.
>
> If we implemented `std::unreachable()` as an empty body, it would result in "unexpected pass".

It //will// work even with an empty body. `[[noreturn]]` already informs the compiler that the function never returns. https://godbolt.org/z/qc97n59Kc. In this example The return value is even 0. There is just no way to verify the behavior. https://godbolt.org/z/fPGEYfEWv returns 1. The test is just too unstable.



================
Comment at: libcxx/test/std/utilities/utility/utility.unreachable/unreachable.compile.pass.cpp:8
+//===----------------------------------------------------------------------===//
+
+#include <utility>
----------------
ldionne wrote:
> This needs some standard requirements.
> 
> I suggest running just the test you're adding locally under various standard modes (`build/bin/llvm-lit -sv libcxx/test/<path-to-test> --param std=c++XX`) to shake out basic failures and get faster turn around time.
Yeah I sometimes forget.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119152/new/

https://reviews.llvm.org/D119152