[llvm] d2bd075 - Fix -Wunused-variable after D80515

Fāng-ruì Sòng via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 8 19:56:02 PDT 2020


On Mon, Jun 8, 2020 at 6:58 PM David Blaikie <dblaikie at gmail.com> wrote:

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

(Many builds enable -Werror, so -Wunused-variable may be an error.)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200608/4275a67e/attachment.html>


More information about the llvm-commits mailing list