[PATCH] D44104: TableGen: Add !foldl operation

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle closed this revision.
nhaehnle marked 2 inline comments as done.
nhaehnle added a comment.

r326790



================
Comment at: docs/TableGen/LangRef.rst:98-101
                :!eq     !if      !head    !tail      !con
                :!add    !shl     !sra     !srl       !and
                :!or     !empty   !subst   !foreach   !strconcat
+               :!cast   !listconcat       !size      !foldl
----------------
tra wrote:
> Should we sort them?
Probably a good idea at some point, but I'd rather not do it in this commit to avoid conflicts with later changes.


================
Comment at: lib/TableGen/TGParser.cpp:1251-1252
+    if (!Start) {
+      TokError("could not get type of !foldl start");
+      return nullptr;
+    }
----------------
tra wrote:
> tra wrote:
> > You seem to mix `fold` and `!foldl` in the error messages throughout the function. Is that intentional? I'd use `!foldl` everywhere for consistency.
> Would it make sense to print `StartUntyped` here, too? I often find TableGen failures baffling, specifically because tablegen will usually (sometimes?) tell you *where* tablegen ran into the problem, but it rarely tells you what exactly it saw after all the folding that made it unhappy. Better error reporting in general should probably be a patch, but I doubt there will be many voices against better error reporting in new code. :-)
Yup, makes sense, I've added that, same with `ListUntyped` below.


================
Comment at: lib/TableGen/TGParser.cpp:1251-1258
+      TokError("could not get type of !foldl start");
+      return nullptr;
+    }
+
+    if (Lex.getCode() != tgtok::comma) {
+      TokError("expected ',' in fold operator");
+      return nullptr;
----------------
nhaehnle wrote:
> tra wrote:
> > tra wrote:
> > > You seem to mix `fold` and `!foldl` in the error messages throughout the function. Is that intentional? I'd use `!foldl` everywhere for consistency.
> > Would it make sense to print `StartUntyped` here, too? I often find TableGen failures baffling, specifically because tablegen will usually (sometimes?) tell you *where* tablegen ran into the problem, but it rarely tells you what exactly it saw after all the folding that made it unhappy. Better error reporting in general should probably be a patch, but I doubt there will be many voices against better error reporting in new code. :-)
> Yup, makes sense, I've added that, same with `ListUntyped` below.
You're right, I've changed all the messages to say `!foldl` consistently.


Repository:
  rL LLVM

https://reviews.llvm.org/D44104