aaron.ballman added a comment.
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.
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
