[PATCH] D99275: Add the TableGen assert statement, step 2

[Chris Lattner via Phabricator via llvm-commits]

lattner accepted this revision.
lattner added a comment.
This revision is now accepted and ready to land.

Looking good, a couple suggestions above.  Also please address teh clang format issues, thanks!



================
Comment at: llvm/include/llvm/TableGen/Error.h:51
 
+LLVM_ATTRIBUTE_NORETURN void CheckAssert(SMLoc Loc, Init *Condition,
+                                                    Init *Message);
----------------
Why is this marked LLVM_ATTRIBUTE_NORETURN?  It looks like it does return.  I think this is copy/pasto?


================
Comment at: llvm/lib/TableGen/Error.cpp:162
+                        Condition->convertInitializerTo(IntRecTy::get()));
+  if (CondValue) {
+    if (!CondValue->getValue()) {
----------------
i'd recommend inverting this conditional (so it is written as `if (!CondValue)` which will allow turning the nested if into an else chain


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99275