[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

I have a few nitpicks, but otherwise this seems right.  I'll wait for the llvm-dev discussion before LGTM'ing though.



================
Comment at: llvm/lib/AsmParser/LLParser.cpp:2930-2931
+    if (NameID != -1 && unsigned(NameID) != NumberedVals.size()) {
+      P.Error(Loc, "label expected to be numbered '%" +
+                       Twine(NumberedVals.size()) + "'");
+      return nullptr;
----------------
Since the label doesn't include a `%`, I think you should drop that part of the error message.


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:2936-2937
+    if (!BB) {
+      P.Error(Loc,
+              "unable to create block numbered " + Twine(NumberedVals.size()));
+      return nullptr;
----------------
I think single quotes around the number would be better here.


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:5580-5581
   switch (Token) {
-  default:                    return Error(Loc, "expected instruction opcode");
+  default:
+    return Error(Loc, "expected instruction opcode");
   // Terminator Instructions.
----------------
Looks unrelated to this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58548