[llvm] r196607 - Integrated assembler incorrectly lexes ARM-style comments

[David Peixotto]

On 1/10/2014 9:28 PM, Saleem Abdulrasool wrote:
> The symref-arm test seems reasonable to me.  I think that
> arm-elf-symver.s or arm-elf-symbol-versioning.s would be a more
> descriptive name, but bike-shedding :-).

Good point. I had copied the elf test which was named symref. I will 
change the name in the next patch.

>     Modifying the Lexer to allow @ in the symbol for that one directive was
>     relatively easy. The harder part was only doing it for ARM in the
>     ELFAsmParser. Any feedback to improve the patch is appreciated.
>
>
> Since this is in the ELF specific parser, and AFAIK, @ is always used
> for symbol versioning in ELF, is there any reason to perform the check
> for the target and then behave accordingly?

I was not sure if every other target used @ for symbol versioning. It 
seems likely, but I did not know if was ok to make that assumption.

> I think you could simply add in an optional parameter to parseIdentifier
> (permitAt) to consume an identifier including the at sign only in the
> symver directive case.  It feels that such a change would be nicer than
> behaving differently on different targets.

The problem is that we need to catch it in the lexer not the parser. I 
do not quite get how that would work. If we add a default parameter of 
true (allowAt), then we would need to explicitly pass it as false for 
ARM in every other location, right?