[PATCH] D91460: [AsmParser] make .ascii support spaces as separators

Jian Cai via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 4 15:03:34 PST 2020


jcai19 added inline comments.


================
Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:3005
 /// parseDirectiveAscii:
-///   ::= ( .ascii | .asciz | .string ) [ "string" ( , "string" )* ]
+///   ::= .ascii [ "string" ( ( , ) "string" )* ]
+///   ::= ( .asciz | .string ) [ "string" ( , "string" )* ]
----------------
jrtc27 wrote:
> Hm, this isn't particularly clear as it stands that multiple strings next to each other is valid, it just looks like the comma has been put it in brackets. Maybe this to better reflect how it's parsed (square brackets around the comma instead would also work, but doesn't capture the logical parse tree so well)? I don't know if the `"string"` needs to be put in parentheses though to apply `+` to it; is this a real BNF-esque grammar or just a hand-wavey one for human consumption?
It appears to be only for human consumption, at least I could not find anywhere it was parsed. For .ascii we could use either space or comma to achieve the same effect, which made comma optional. Based on the original text, I assume brackets mean the enclosed symbol is mandatory while parentheses means it is optional. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91460



More information about the llvm-commits mailing list