inclyc added a comment.
In D135551#3846592 <https://reviews.llvm.org/D135551#3846592>, @aaron.ballman
wrote:
> I don't know if that discussion reached a conclusion to move forward with
> this change -- my reading of the conversation was that efforts would be
> better spend on fuzzing instead of changing policy about using unreachable vs
> assert(0).
>
> In general, I'm a bit hesitant to make this change. On the one hand, it's
> logically no worse than using `assert(0)` in a release build (if you hit this
> code path, bad things are going to happen). But `__builtin_unreachable` can
> have time travel optimization effects that `assert` doesn't have, and so the
> kind of bad things which can happen are different between the two (and use of
> unreachable on reachable code paths might make for harder debugging in
> RelWithDebInfo builds). Historically, we've usually used `llvm_unreachable`
> for situations where we're saying "this code cannot be reached; if it can,
> something else has gone seriously wrong." For example, in code like: `int
> foo(SomeEnum E) { switch (E) { case One: return 1; default: return 2; }
> llvm_unreachable("getting here would be mysterious"); }` and we've used
> `assert(0)` for situations where we're saying "this code is possible to reach
> only when there were mistakes elsewhere which broke invariants we were
> relying on." The two situations are similar, but still different enough that
> I don't think we should wholesale change from one form to another.
> but still different enough that I don't think we should wholesale change from
> one form to another.
In general we can control the behavior here via `-DLLVM_UNREACHABLE_OPTIMIZE`
to choose making assumptions or traps (looks better than assertions to me).
https://github.com/llvm/llvm-project/blob/50312ea133999cb2aad1ab9ef0ec39429a9427c5/llvm/include/llvm/Support/ErrorHandling.h#L125
(This change was landed 7 months ago https://reviews.llvm.org/D121750)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135551/new/
https://reviews.llvm.org/D135551
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
