[llvm-commits] patch: refactor the asmparser a little

Nick Lewycky nicholas at mxc.ca
Sat Jun 4 11:24:28 PDT 2011


Rafael Ávila de Espíndola wrote:
> On 11-06-02 8:18 PM, Nick Lewycky wrote:
>> The attached patch refactors the asm parsers' parsing of strings into a
>> ReadString method and parsing of LocalVarName or GlobalVarName into
>> ReadName(). No functionality change intended.
>>
>> I'm asking for review because the parser doesn't already have helper
>> functions like these, and I wanted to check whether the error-reporting
>> style (inconsistent between the two) is okay to commit or if anyone has
>> suggestions.
>
> I am not familiar with this code, but the patch looks reasonable. The
> inconsistency is probably OK, since the functions have different uses.

Good point.

> If you want to be a bit more consistent, you could do:
>
> lltok::Kind kind;
> if (ReadString(lltok::StringConstant,&kind))
>     return kind;

Mm, that doesn't solve the real problem which is that we want to report 
error (Error, Eof) which are token kinds, but don't yet know what token 
it is in the non-error case by the time ReadString has finished. In 
particular, that token could end up being a LabelStr, not a StringConstant.

Thanks so much for the code review!

Nick



More information about the llvm-commits mailing list