[PATCH] D32701: [LLVM][inline-asm][Altmacor] Altmacro string delimiter '<..>'

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

With that in mind, I think you should then go back to your original implementation (ignore quotes) and let the expression evaluation crash if there's any single quote in between (or whatever is the expected behaviour).

Maybe we should, then, add a comment on that function, saying the docs mention single-quotes but GCC doesn't support, so we won't either.

Sorry for the confusion.

cheers,
--renato



================
Comment at: lib/MC/MCParser/AsmParser.cpp:1201
+/// character.
+bool AsmParser::isAltmacroString(SMLoc &StrLoc, SMLoc &EndLoc) {
+  assert((StrLoc.getPointer() != NULL) &&
----------------
m_zuckerman wrote:
> rengolin wrote:
> > rengolin wrote:
> > > I'd add single quotes at the same time.
> > > 
> > > This seems like a simple addition, as you can pass the expected "ending char" as an argument, in addition to `\r\n\0`.
> > The problem here, IIUC, is that you could start with '<' and end with single-quote (or vice-versa) and this code would accept. Also, if you start with '<', single-quote isn't actually a terminator.
> > 
> > To make sure this works, you have to test four overall patterns:
> >   - <foo,bar>
> >   - <foo'bar>
> >   - 'foo,bar'
> >   - 'foo>bar'
> > 
> > They all have to produce a single string "foo?bar".
> > 
> > What I meant was something like:
> > 
> >     bool AsmParser::isAltmacroString(SMLoc &StrLoc, SMLoc &EndLoc, const char Term) {
> >       ...
> >       while ((*CharPtr != Term) && (*CharPtr != '\n') && (*CharPtr != '\r') && (*CharPtr != '\0')) {
> >         CharPtr++;
> >     }
> > 
> > And then down there, something like:
> > 
> >     } else if (Lexer.IsaAltMacroMode() && Lexer.is(AsmToken::Less) && Lexer.is(AsmToken::SingleQuote) &&
> >                isAltmacroString(StrLoc, EndLoc, Lex()) {
> > 
> > or something.
> >     
> > 
> 1. The code will not accept it because the last character that I check is '>'. So if the code ends with "\ '". The function returns false since it does not equal to '>' as required by the second if.
> 
> 2. regard the <foo'bar>:  I don't really sure what is the usage of it. 
> GAS doesn't really support the "\'" delimiter and the only full support that I found using https://godbolt.org/ was for the "<...>".  
> 
> 3. I prefer only to support '<..>' as in GCC. And I don't mind to define single quote as a string in altmacro, but it must end with a single quote.
> 
Oh, I was not aware the single quotes were unsupported in GCC. That's what I get for trusting their docs... :)


https://reviews.llvm.org/D32701