[PATCH] D131337: Wrap `llvm_unreachable` macro in do-while loop

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/include/llvm/Support/ErrorHandling.h:150
 #else
-#define llvm_unreachable(msg) LLVM_BUILTIN_TRAP, LLVM_BUILTIN_UNREACHABLE
+#define llvm_unreachable(msg) \
+  do { LLVM_BUILTIN_TRAP; LLVM_BUILTIN_UNREACHABLE; \
----------------
sgraenitz wrote:
> barannikov88 wrote:
> > This is improperly formatted, please use clang-format.
> Right, clang-format want's this:
> ```
>   do { \
>     LLVM_BUILTIN_TRAP; \
>     LLVM_BUILTIN_UNREACHABLE; \
>   } while (false)
> ```
> 
> I choose not to follow it in my patch, because the existing code is only 6 months old and didn't respect clang-format. Instead I mimicked the style of the `LLVM_DEBUG` macro which expands into a similar do-while construct: https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/Debug.h#L64
> 
> Of course, I can change that if there are good reasons or strong opinions on it.
Imagine that the 6-month old change followed the style. It would save your time on deciding whether to clang-format it or not. Formatting it properly now will save people time when modifying this file in the future. This is the whole purpose of the style guide -- saving time on choosing the right formatting.
I can't disagree that consistency is more important than style, but LLVM_DEBUG is not even in this file.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131337