From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 108020 invoked by alias); 1 Sep 2017 12:57:50 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 108007 invoked by uid 89); 1 Sep 2017 12:57:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 01 Sep 2017 12:57:44 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 52D61C0467D8; Fri, 1 Sep 2017 12:57:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 52D61C0467D8 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@redhat.com Received: from redhat.com (ovpn-204-79.brq.redhat.com [10.40.204.79]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 06609A63AB; Fri, 1 Sep 2017 12:57:41 +0000 (UTC) Date: Fri, 01 Sep 2017 12:57:00 -0000 From: Marek Polacek To: GCC Patches Cc: Joseph Myers , Jason Merrill , David Malcolm Subject: Re: c-family PATCH to improve -Wtautological-compare (PR c/81783) Message-ID: <20170901125739.GU17069@redhat.com> References: <20170816142917.GB17069@redhat.com> <1502896056.3741.17.camel@redhat.com> <20170816152456.GD17069@redhat.com> <20170825124745.GA17069@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170825124745.GA17069@redhat.com> User-Agent: Mutt/1.8.3 (2017-05-23) X-SW-Source: 2017-09/txt/msg00043.txt.bz2 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: > > > > .  > > > > > > 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