[PATCH] D145872: TableGen: Let expressions available to list subscripts and list slices

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: llvm/docs/TableGen/ProgRef.rst:348
+               :| `Value` "-" `Value`
+               :| `Value` `TokInteger`(negative)
 
----------------
IIUC this is supposed to be normative and `(negative)` is not normative.



================
Comment at: llvm/include/llvm/TableGen/Record.h:852
+    LISTSLICE,
+    RANGEC,
     STRCONCAT,
----------------
What is C in RANGEC? Please give a descriptive name.


================
Comment at: llvm/lib/TableGen/TGParser.cpp:746
+  }
+  case tgtok::IntVal: { // Deprecated "-num"
+    auto i = -Lex.getCurIntVal();
----------------
I don't get it how IntVal implies `-`. Could you explain?


================
Comment at: llvm/lib/TableGen/TGParser.cpp:788-789
+  TypedInit *CurVal;
+  SmallVector<Init *, 2> LoV;      // int
+  SmallVector<TypedInit *, 2> LoL; // list<int>
+
----------------
Please give entities descriptive / canonical names.



================
Comment at: llvm/lib/TableGen/TGParser.cpp:2675-2678
 ///   Value       ::= SimpleValue ValueSuffix*
 ///   ValueSuffix ::= '{' BitList '}'
-///   ValueSuffix ::= '[' BitList ']'
+///   ValueSuffix ::= '[' SliceElements ']'
 ///   ValueSuffix ::= '.' ID
----------------
The whole comment should be synced with the documentation.



================
Comment at: llvm/lib/TableGen/TGParser.cpp:2718
+      auto *LHS = dyn_cast<TypedInit>(Result);
+      assert(LHS);
+      auto *LHSTy = dyn_cast<ListRecTy>(LHS->getType());
----------------
ParseSimpleValue is not guaranteed to return TypedInit, this should be an error.



================
Comment at: llvm/lib/TableGen/TGParser.cpp:2721-2722
+      if (!LHSTy) {
+        Error(SquareLoc, "Type '" + Twine(LHS->getType()->getAsString()) +
+                             "' is invalid for list subscript");
+        return nullptr;
----------------
It should point to the element, not the opening bracket.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145872