Fix for pr13145: Naming a function like a register name confuses the asm parser
Chad Rosier
mcrosier at apple.com
Tue Mar 19 16:52:57 PDT 2013
Hi Stepan,
I went ahead and applied your patch in r177463.
Chad
On Mar 18, 2013, at 9:41 AM, Chad Rosier <mcrosier at apple.com> wrote:
>
> 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>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130319/f33cd167/attachment.html>
More information about the llvm-commits
mailing list