This is an archive of the discontinued LLVM Phabricator instance.

Clarify use of llvm_unreachable in the coding standard
ClosedPublic

Authored by aaron.ballman on Mar 24 2020, 12:21 PM.

Details

Summary

There has been some ongoing confusion regarding when to use llvm_unreachable which this patch attempts to address. Specifically, the confusion has been around whether llvm_unreachable is intended to mark only unreachable code paths that the compiler cannot determine itself or to mark a code path which is unconditionally a bug to reach. Based on email and IRC discussions, it sounds like "unconditional bug to reach" is the consensus.

Diff Detail

Event Timeline

aaron.ballman created this revision.Mar 24 2020, 12:21 PM
dblaikie accepted this revision.Mar 24 2020, 12:36 PM

ah, good to see there's already some verbiage here that covers the general idea (which I'd clearly forgotten about, might've been good framing for the discussion earlier, so good to know it's here) - specifically saying "here's a better thing" but "use it" is left as an implication - the new paragraph makes that more explicit, which is good.

Thanks a bunch!

This revision is now accepted and ready to land.Mar 24 2020, 12:36 PM
hubert.reinterpretcast added inline comments.
llvm/docs/CodingStandards.rst
1160

Suggestion:
[ ... ] bug<ins> (not originating from user input; see below)</ins> of some kind. [ ... ]

Also, it would help for the patch to include more context.

Updated based on review feedback, showing more context as well.

aaron.ballman marked 2 inline comments as done.Mar 24 2020, 1:00 PM
aaron.ballman added inline comments.
llvm/docs/CodingStandards.rst
1160

Done, and thank you for the reminder about diff context.

joerg accepted this revision.Mar 24 2020, 1:05 PM

This summarizes the IRC discussion.

aaron.ballman closed this revision.Mar 26 2020, 5:09 AM
aaron.ballman marked an inline comment as done.

Thanks for the feedback, I've commit in 4778e409de112dc4b70933f3e3f502d6c747ed5a