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

Joerg Sonnenberger joerg at britannica.bec.de
Tue Jan 14 04:25:05 PST 2014


On Mon, Jan 13, 2014 at 06:07:13PM -0800, David Peixotto wrote:
> On 1/12/2014 12:36 PM, Joerg Sonnenberger wrote:
> >On Fri, Jan 10, 2014 at 06:24:19PM -0800, David Peixotto wrote:
> >>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.
> >
> >What about using two flags. The basic AllowAtInIdentifiers as static and
> >derived from the comment char. A second flag for AllowAtInNextIdentifier
> >which is only set by .symvar and reset again. That completely removes
> >the need to special case ARM.
> 
> I think we could accomplish the same thing by adding a
> getAllowAtInIdentifier() method and use that to query the value so
> that we can set it unconditionally and the restore it to the same
> state. I think that would be cleaner than having two bits of state
> for the same setting.

Providing get/set works as well. I have no opinion on one vs the other.

> 
> Do you prefer that approach over my latest patch?

The latest patch is using isARM? That's a clear no-go, IMO.

Joerg



More information about the llvm-commits mailing list