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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini 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; \
----------------
barannikov88 wrote:
> 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.
> 
FYI: I find the clang-format version much more readable, it's not clear to me why we shouldn't just adopt it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131337