[llvm] r223142 - Appease a build bot complaining about an unused variable that's used in an assertion.

Aaron Ballman aaron at aaronballman.com
Tue Dec 2 13:07:17 PST 2014


On Tue, Dec 2, 2014 at 4:03 PM, Philip Reames <listmail at philipreames.com> wrote:
>
> On 12/02/2014 12:53 PM, Philip Reames wrote:
>>
>>
>> On 12/02/2014 11:47 AM, Tom Stellard wrote:
>>>
>>> On Tue, Dec 02, 2014 at 02:38:27PM -0500, Aaron Ballman wrote:
>>>>
>>>> On Tue, Dec 2, 2014 at 2:28 PM, Philip Reames
>>>> <listmail at philipreames.com> wrote:
>>>>>
>>>>> Author: reames
>>>>> Date: Tue Dec  2 13:28:57 2014
>>>>> New Revision: 223142
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=223142&view=rev
>>>>> Log:
>>>>> Appease a build bot complaining about an unused variable that's used in
>>>>> an assertion.
>>>>>
>>>>>
>>>>> Modified:
>>>>> llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>>>>>
>>>>> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp?rev=223142&r1=223141&r2=223142&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp
>>>>> (original)
>>>>> +++ llvm/trunk/lib/CodeGen/SelectionDAG/StatepointLowering.cpp Tue Dec
>>>>> 2 13:28:57 2014
>>>>> @@ -230,6 +230,7 @@ static SDNode *lowerCallFromStatepoint(c
>>>>>
>>>>>     int NumCallArgs =
>>>>> dyn_cast<ConstantInt>(CI.getArgOperand(1))->getZExtValue();
>>>>>     assert(NumCallArgs >= 0 && "non-negative");
>>>>> +  (void)NumCallArgs;
>>>>
>>>> I think it's preferable to remove the variable entirely, and instead
>>>> write:
>>>>
>>>> assert(dyn_cast<ConstantInt>(CI.getArgOperand(1))->getZExtValue() >= 0
>>>> && "non-negative");
>>>>
>>>> This negates the ugly workaround, and also reduces overhead in builds
>>>> without asserts.
>>>>
>>> Also shouldn't it be cast instead of dyn_cast.
>>
>> I'm preparing a patch that restructures this code.  The intent of the
>> assert is best served with a stronger check in an earlier location and the
>> removal of this assert entirely.  I'll submit that shortly.
>
> Committed revision 223150
>
> Let me know if that doesn't address the concerns.

LGTM, thanks!

~Aaron



More information about the llvm-commits mailing list