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