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

David Peixotto dpeixott at codeaurora.org
Tue Jan 14 10:54:00 PST 2014


On 1/14/2014 4:25 AM, Joerg Sonnenberger wrote:
> 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.

Ok, I will post a patch using that approach.

>>
>> Do you prefer that approach over my latest patch?
>
> The latest patch is using isARM? That's a clear no-go, IMO.

No, sorry I meant a later patch that got rid of isARM. Renato objected 
to it for other reasons. Will post an updated patch soon.



More information about the llvm-commits mailing list