This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: c-family PATCH to improve -Wtautological-compare (PR c/81783)
- From: Marek Polacek <polacek at redhat dot com>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>
- Cc: Joseph Myers <joseph at codesourcery dot com>, Jason Merrill <jason at redhat dot com>, David Malcolm <dmalcolm at redhat dot com>
- Date: Fri, 1 Sep 2017 14:57:39 +0200
- Subject: Re: c-family PATCH to improve -Wtautological-compare (PR c/81783)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx07.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx07.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=polacek at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 52D61C0467D8
- References: <20170816142917.GB17069@redhat.com> <1502896056.3741.17.camel@redhat.com> <20170816152456.GD17069@redhat.com> <20170825124745.GA17069@redhat.com>
Ping.
On Fri, Aug 25, 2017 at 02:47:45PM +0200, Marek Polacek wrote:
> Ping.
>
> On Wed, Aug 16, 2017 at 05:24:56PM +0200, Marek Polacek wrote:
> > On Wed, Aug 16, 2017 at 11:07:36AM -0400, David Malcolm wrote:
> > > On Wed, 2017-08-16 at 16:29 +0200, Marek Polacek wrote:
> > > > This patch improves -Wtautological-compare so that it also detects
> > > > bitwise comparisons involving & and | that are always true or false,
> > > > e.g.
> > > >
> > > > if ((a & 16) == 10)
> > > > return 1;
> > > >
> > > > can never be true. Note that e.g. "(a & 9) == 8" is *not* always
> > > > false
> > > > or true.
> > > >
> > > > I think it's pretty straightforward with one snag: we shouldn't warn
> > > > if
> > > > the constant part of the bitwise operation comes from a macro, but
> > > > currently
> > > > that's not possible, so I XFAILed this in the new test.
> > >
> > > Maybe I'm missing something here, but why shouldn't it warn when the
> > > constant comes from a macro?
> >
> > Just my past experience. Sometimes you can't really control the macro
> > and then you get annoying warnings.
> >
> > E.g. I had to tweak the warning that warns about if (i == i) to not warn about
> >
> > #define N 2
> > if (a[N] == a[2]) {}
> >
> > because that gave bogus warning during bootstrap, if I recall well.
> >
> > > At the end of your testcase you have this example:
> > >
> > > #define N 0x10
> > > if ((a & N) == 10) /* { dg-bogus "bitwise comparison always evaluates to false" "" { xfail *-*-* } } */
> > > return 1;
> > > if ((a | N) == 10) /* { dg-bogus "bitwise comparison always evaluates to false" "" { xfail *-*-* } } */
> > > return 1;
> > >
> > > That code looks bogus to me (and if the defn of "N" is further away,
> > > it's harder to spot that it's wrong): shouldn't we warn about it?
> >
> > I'm glad you think so. More than happy to make it an expected warning.
> >
> > > > This has found one issue in the GCC codebase and it's a genuine bug:
> > > > <https://gcc.gnu.org/ml/gcc-patches/2017-08/msg00757.html>.
> > >
> > > In this example GOVD_WRITTEN is from an enum, not a macro, but if
> > > GOVD_WRITTEN had been a macro, shouldn't we still issue a warning?
> >
> > I feel like we should, but some might feel otherwise.
> >
> > Thanks,
> >
> > Marek
>
> Marek
Marek