Fix for pr13145: Naming a function like a register name confuses the asm parser
Chad Rosier
mcrosier at apple.com
Mon Mar 18 09:41:57 PDT 2013
On Mar 18, 2013, at 1:32 AM, Stepan Dyatkovskiy <stpworld at narod.ru> wrote:
> Hello Chad.
> Sorry about that, a little bit confused by ogre3d style. Variable name was fixed. I also replaced the comment.
> Unittest is applied.
Great, thanks.
+ // If we've seen a branch mnemonic, the next operand must be a label then.
+ // Even if it named as register. So "br r1" would mean:
+ // branch to "r1" label.
Perhaps this comment:
// If we've seen a branch mnemonic, the next operand must be a label.
// This is true even if the label is a register name. So "br r1" means
// branch to label "r1."
If you have commit access, please go ahead and commit the patch. Otherwise, let me know and I'll commit on you behalf.
Chad
> -Stepan.
>
> Chad Rosier wrote:
>> Hi Stepan,
>> Here are a few nits:
>>
>> When writing comments, write them as English prose, which means they
>> should use proper capitalization, punctuation, etc.
>>
>> + // Want label whatever happened...
>> + bool wantLabelOnly = Mnemonic == "b" || Mnemonic == "bl";
>>
>> Maybe the comment could be something like, "If we've seen a branch
>> mnemonic, the next operand must be a label."
>>
>> You could also rename wantLabelOnly to ExpectLabel or just put the
>> conditional in the if statement; the comment should make clear the
>> condition of the if statement.
>>
>> Most importantly, please add a test case before resubmitting to the list.
>>
>> Chad
>
> <pr13145.patch>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130318/c2d82059/attachment.html>
More information about the llvm-commits
mailing list