<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 8, 2020 at 6:58 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Mon, Jun 8, 2020 at 6:51 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>> wrote:<br>
><br>
> "Remove unused variable (-Wunused-variable in D80515)"<br>
><br>
> Doesn't "unused variable" repeat the information conveyed by -Wunused-variable?<br>
<br>
"Remove" is the important bit that I find missing from many patches<br>
like this (not only yours) - up to you if you want to include how this<br>
unused variable was found (by -Wunused-variable rather than by some<br>
other method) though I don't think it's super important how it's<br>
found, we probably all agree that removing unused variable is good,<br>
even if it were found by visual inspection, rather than by a warning.<br>
<br>
> About the preposition "after", I saw many people used it, so I stick with it.<br>
> If it is not proper, I can change for future changes.<br>
<br>
Oh, no, didn't mean to draw any particular distinction in that word<br>
choice. If I had more space for words I'd say "introduced by" or<br>
"introduced in".<br>
<br>
><br>
> On Mon, Jun 8, 2020 at 6:29 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
>><br>
>> Could you describe the nature of the change in the subject line? Makes<br>
>> it easier to filter on what is actually changing, rather than what<br>
>> motivated the change (the warning). (I know this can seem at odds with<br>
>> the usual advice to describe the purpose of a change moreso than the<br>
>> mechanical details of a change (since the mechanical details can be<br>
>> seen in the associated diff) - but "Remove unused variable<br>
>> (-Wunused-variable in D80515)" probably covers all of it?)<br>
>><br>
>> On Fri, Jun 5, 2020 at 11:47 AM Fangrui Song via llvm-commits<br>
>> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>> ><br>
>> ><br>
>> > Author: Fangrui Song<br>
>> > Date: 2020-06-05T11:46:50-07:00<br>
>> > New Revision: d2bd075e8d1f4544357c0ab9784ec4e88bd229a7<br>
>> ><br>
>> > URL: <a href="https://github.com/llvm/llvm-project/commit/d2bd075e8d1f4544357c0ab9784ec4e88bd229a7" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/d2bd075e8d1f4544357c0ab9784ec4e88bd229a7</a><br>
>> > DIFF: <a href="https://github.com/llvm/llvm-project/commit/d2bd075e8d1f4544357c0ab9784ec4e88bd229a7.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/d2bd075e8d1f4544357c0ab9784ec4e88bd229a7.diff</a><br>
>> ><br>
>> > LOG: Fix -Wunused-variable after D80515<br>
>> ><br>
>> > Added:<br>
>> ><br>
>> ><br>
>> > Modified:<br>
>> >     llvm/lib/Target/ARM/ARMISelLowering.cpp<br>
>> ><br>
>> > Removed:<br>
>> ><br>
>> ><br>
>> ><br>
>> > ################################################################################<br>
>> > diff  --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp<br>
>> > index 10e7137f4e80..e4805eedbda5 100644<br>
>> > --- a/llvm/lib/Target/ARM/ARMISelLowering.cpp<br>
>> > +++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp<br>
>> > @@ -14460,7 +14460,6 @@ static SDValue PerformVMOVNCombine(SDNode *N,<br>
>> >  static SDValue PerformVQMOVNCombine(SDNode *N,<br>
>> >                                      TargetLowering::DAGCombinerInfo &DCI) {<br>
>> >    SDValue Op0 = N->getOperand(0);<br>
>> > -  SDValue Op1 = N->getOperand(1);<br>
>> >    unsigned IsTop = N->getConstantOperandVal(2);<br>
>> ><br>
>> >    unsigned NumElts = N->getValueType(0).getVectorNumElements();<br><br></blockquote><div> </div></div><div dir="ltr">Thanks for the feedback! Will use 'remove unused variables' in the future. 'introduced after' seems long. I'll be lazy and stick with 'after'<br></div><div dir="ltr"><br></div><div>(Many builds enable -Werror, so -Wunused-variable may be an error.)</div></div>