[libcxx-commits] [PATCH] D147860: [libcxxabi] [test] Mark code following an assert(false) as unreachable

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxxabi/test/cxa_vec_new_overflow_PR41395.pass.cpp:18-19
 
 void dummy_ctor(void*) { assert(false && "should not be called"); }
 void dummy_dtor(void*) { assert(false && "should not be called"); }
 
----------------
phosek wrote:
> Should we make the same change to all other `dummy` functions for consistency?
I guess that could be good, especially wrt the arguments about uses of unreachable that you mentioned.


================
Comment at: libcxxabi/test/cxa_vec_new_overflow_PR41395.pass.cpp:21
 
-void *dummy_alloc(size_t) { assert(false && "should not be called"); }
+void *dummy_alloc(size_t) { assert(false && "should not be called"); __builtin_unreachable(); }
 void dummy_dealloc(void*) { assert(false && "should not be called"); }
----------------
phosek wrote:
> I wonder if we should instead use `__bultin_trap()` to ensure that the code doesn't continue executing even with assertions disabled? This was also discussed in https://discourse.llvm.org/t/llvm-unreachable-is-widely-misused/60587.
I guess that's a reasonable fix too, I'll try that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147860