[PATCH] D91460: [AsmParser] make .ascii/.asciz/.string support multiple strings

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added a comment.

This needs tests, both to show it accepts multiple strings and to show for each of the three directives that it produces the right output (i.e. is equivalent to multiple individual directives).



================
Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:3017-3019
+  while (true) {
+    if (parseOptionalToken(AsmToken::EndOfStatement)) // return true on success
+      break;
----------------



================
Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:3020
+      break;
+    if (parseOp()) // return false on success
+      return addErrorSuffix(" in '" + Twine(IDVal) + "' directive");
----------------
Should be "returns" not "return" otherwise it means something very different, though I'm not sure this is a particularly useful comment. You could also consider inverting the return value of the parseOp lambda above to be more intuitive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91460