[PATCH] D43754: TableGen: Explicitly check whether a record has been resolved

Artem Belevich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 26 16:23:18 PST 2018


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

Few minor nits/suggestions. LGTM otherwise.



================
Comment at: lib/TableGen/TGParser.cpp:75
+    Init *Bit = BV->getBit(i);
+    bool Ok = false;
+    if (auto VBI = dyn_cast<VarBitInit>(Bit)) {
----------------
Nit. `HasValue` would be more descriptive. OK is too generic. HasValue gives a hint what exactly do we consider to be OK.


================
Comment at: lib/TableGen/TGParser.cpp:84
+    }
+    if (!Ok && !Bit->isConcrete())
+      return false;
----------------
This may be my personal preference, but I find it easier to grasp expressions with smaller number of negations. E.g. for me `if(!(Ok || Bit->isConcrete()))` is easier for to understand without having to stop and think about it.


================
Comment at: lib/TableGen/TGParser.cpp:100-104
+      bool Ok;
+      if (isa<BitsInit>(V))
+        Ok = checkBitsConcrete(R, RV);
+      else
+        Ok = V->isConcrete();
----------------
I'd fold it into `bool OK = isa<BitsInit>(V)) ? checkBitsConcrete(R, RV) : V->isConcrete();`



Repository:
  rL LLVM

https://reviews.llvm.org/D43754





More information about the llvm-commits mailing list