[llvm-commits] [LLVM, llvm-mc] bugfix for bug #10869: Unclear error for files without newline at the end of file (ARM, x86).

[Stepan Dyatkovskiy]

Hi,
So, is this patch ok?

Regards,
Stepan.

12.09.2011, 22:04, "Stepan Dyatkovskiy" <STPWORLD at yandex.ru>:
> Hi,
> It is a commonly encountered problem for AsmParser and for all its extensions (like an ARMAsmParser or MBlazeAsmParser).
> Now the finishing of instruction parsing in extensions looks like this:
> ARMAsmParser::ParseInstruction(...) {
>  //...
>   if (Token == EndOfStatement) {
>       // finish parse instruction
>   }
> }
>
> In a case of your suggestion fix will look like this:
> ARMAsmParser::ParseInstruction(...) {
>   //...
>   if (Token == EndOfStatement  /*the fix: */ && Token == EOF) {
>       // finish parse instruction
>   }
> }
>
> I think that for AsmParser extensions EOF means also End-Of-Statment. So probably it will be more comfortable for them to see the EndOfStatement first and only after it EOF?
>
> Stepan
>
> 09.09.2011, 20:22, "James Molloy" <james.molloy at arm.com>:
>
>>  Hi,
>>
>>  Wouldn't it be better to fix the parsers themselves to handle EOF correctly? I
>>  find it slightly unintuitive to have EOF cause two tokens to be emitted -
>>  EndOfStatement and EOF.
>>
>>  Or is this not practicable for some reason?
>>
>>  Cheers,
>>
>>  James
>>>   -----Original Message-----
>>>   From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
>>>   bounces at cs.uiuc.edu] On Behalf Of Stepan Dyatkovskiy
>>>   Sent: 09 September 2011 14:05
>>>   To: llvm-commits at cs.uiuc.edu
>>>   Subject: Re: [llvm-commits] [LLVM, llvm-mc] bugfix for bug #10869: Unclear
>>>   error for files without newline at the end of file (ARM, x86).
>>>
>>>   ping...
>>>
>>>   P.S:
>>>   I reattached the patch. In a case of some problems with previous
>>>   attachment downloading, use this one. Thats why I used another email
>>>   account.
>>>
>>>   07.09.2011, 03:44, "Stepan Dyatkovskiy" <STPWORLD at yandex.ru>:
>>>>   Hello everyone,
>>>>
>>>>   Please find attached a patch for review.
>>>>   It fixes bug #10869 and adds a regression test for it.
>>>>
>>>>   Best regards,
>>>>   Stepan.
>>>>
>>>>   _______________________________________________
>>>>   llvm-commits mailing list
>>>>   llvm-commits at cs.uiuc.edu
>>>>   http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>  _______________________________________________
>>  llvm-commits mailing list
>>  llvm-commits at cs.uiuc.edu
>>  http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits