[PATCH] D46699: [ThinLTO] Print module summary index to assembly

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 25 15:05:59 PDT 2018


pcc added a comment.

Mostly looking good, a few last comments.



================
Comment at: lib/AsmParser/LLParser.cpp:741
+    case lltok::Eof:
+      return TokError("found end of file while parsing summary entry");
+    default:
----------------
Can you write tests for the parse failures?


================
Comment at: lib/AsmParser/LLParser.cpp:748
+  } while (NumOpenParen > 0);
+  if (!FoundSomeParens)
+    return TokError(
----------------
This code is reachable only if the first token after the "label" is not an lparen, no? There also seems to be a bug: this code will accept `foo: ) (`. Maybe a more straightforward way to write this is:
- expect the lparen
- do the same parsing loop as before, but with NumOpenParen initialized to 1 and no FoundSomeParens variable


Repository:
  rL LLVM

https://reviews.llvm.org/D46699





More information about the llvm-commits mailing list