Clarify use of llvm_unreachable in the coding standard

[aaron.ballman]

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.

[dblaikie]

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!

[hubert.reinterpretcast]

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.

[aaron.ballman]

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

[aaron.ballman]

llvm/docs/CodingStandards.rst

1160

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

[joerg]

This summarizes the IRC discussion.

[aaron.ballman]

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

llvm/docs/CodingStandards.rst

.. code-block:: c++
   llvm_unreachable("Invalid radix for integer literal");
 
 When assertions are enabled, this will print the message if it's ever reached
 and then exit the program. When assertions are disabled (i.e. in release
 builds), ``llvm_unreachable`` becomes a hint to compilers to skip generating
 code for this branch. If the compiler does not support this, it will fall back
 to the "abort" implementation.
 
+Use ``llvm_unreachable`` to mark a specific point in code that should never be
+reached. This is especially desirable for addressing warnings about unreachable
+branches, etc., but can be used whenever reaching a particular code path is
+unconditionally a bug (not originating from user input; see below) of some kind.
+Use of ``assert`` should always include a testable predicate (as opposed to
+``assert(false)``).
+
 Neither assertions or ``llvm_unreachable`` will abort the program on a release
 build. If the error condition can be triggered by user input then the
 recoverable error mechanism described in :doc:`ProgrammersManual` should be
 used instead. In cases where this is not practical, ``report_fatal_error`` may
 be used.
 
 Another issue is that values used only by assertions will produce an "unused
 value" warning when assertions are disabled.  For example, this code will warn: