From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 58107 invoked by alias); 15 Sep 2015 14:34:06 -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 58095 invoked by uid 89); 15 Sep 2015 14:34:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-qg0-f50.google.com Received: from mail-qg0-f50.google.com (HELO mail-qg0-f50.google.com) (209.85.192.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 15 Sep 2015 14:34:04 +0000 Received: by qgev79 with SMTP id v79so143966852qge.0 for ; Tue, 15 Sep 2015 07:34:02 -0700 (PDT) X-Received: by 10.140.235.147 with SMTP id g141mr32760176qhc.22.1442327642465; Tue, 15 Sep 2015 07:34:02 -0700 (PDT) Received: from [192.168.0.26] (97-122-175-227.hlrn.qwest.net. [97.122.175.227]) by smtp.gmail.com with ESMTPSA id s64sm8066915qgs.33.2015.09.15.07.34.00 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 15 Sep 2015 07:34:01 -0700 (PDT) Message-ID: <55F82C57.2080309@gmail.com> Date: Tue, 15 Sep 2015 14:56:00 -0000 From: Martin Sebor User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Mark Wielaard CC: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Warn when comparing nonnull arguments to NULL in a function. References: <1441835087-14555-1-git-send-email-mjw@redhat.com> <55F79274.5080602@gmail.com> <1442305942.8165.334.camel@bordewijk.wildebeest.org> In-Reply-To: <1442305942.8165.334.camel@bordewijk.wildebeest.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg01085.txt.bz2 On 09/15/2015 02:32 AM, Mark Wielaard wrote: > On Mon, 2015-09-14 at 21:37 -0600, Martin Sebor wrote: >>> +void foo(void *bar) __attribute__((nonnull(1))); >>> + >>> +void foo(void *bar) { if (!bar) abort(); } /* { dg-warning "null" "argument ‘bar’ compared to NULL" } */ >> >> This looks like a very useful enhancement. Since the change is limited >> to build_binary_op in the two front ends I wonder if the warning also >> issued for other expressions? For example, suppose I were to add to >> function foo above the following: >> >> bool is_null = bar; >> >> would GCC issue a warning? The same question goes for other expressions >> non-binary expressions, including: >> >> bar ? f () : g (); > > Yes, it will: > > nt.c: In function ‘tf’: > nt.c:9:3: warning: nonnull argument ‘bar’ compared to NULL [-Wnonnull] > bool is_null = bar; > ^ > > nt.c:14:7: warning: nonnull argument ‘bar’ compared to NULL [-Wnonnull] > bar ? f () : g (); > ^ > >> or in C++: >> >> bool x = static_cast(bar); > > Likewise for the g++ frontend: Great! > > nt.c: In function ‘bool tf(void*)’: > nt.c:12:18: warning: nonnull argument ‘bar’ compared to NULL [-Wnonnull] > bool is_null = bar; > ^ > nt.c:14:19: warning: nonnull argument ‘bar’ compared to NULL [-Wnonnull] > bar ? f () : g (); > ^ IME, the C++ diagnostics aren't entirely consistent in where they put the location but this one seems worse than I would expect. It should point at bar. OTOH, here are a couple of examples of the inconsistency, one with the same problem as yours, so it's probably not something you did: $ cat t.c && g++ -c t.c int bar (void *p) { return p < 0.0 ? 0 : 1; struct A { } a; return a ? 0 : 1; } t.c: In function ‘int bar(void*)’: t.c:2:16: error: invalid operands of types ‘void*’ and ‘double’ to binary ‘operator<’ return p < 0.0 ? 0 : 1; ^ t.c:4:20: error: could not convert ‘a’ from ‘bar(void*)::A’ to ‘bool’ return a ? 0 : 1; ^ > nt.c:16:33: warning: nonnull argument ‘bar’ compared to NULL [-Wnonnull] > bool x = static_cast(bar); > ^ > > Although I now notice they differ on the placement of the carrot. > Maybe the location passed into the warning is not correct/ideal? I noticed the same problem in other g++ diagnostics. For instance, in the messages below, the column is that of the closing parenthesis rather than that of the operand: $ cat t.c && g++ -c t.c int foo (int, int); int bar (void *p) { foo (p, 0); return static_cast(p); } t.c: In function ‘int bar(void*)’: t.c:4:14: error: invalid conversion from ‘void*’ to ‘int’ [-fpermissive] foo (p, 0); ^ t.c:1:5: note: initializing argument 1 of ‘int foo(int, int)’ int foo (int, int); ^ t.c:5:30: error: invalid static_cast from type ‘void*’ to type ‘int’ return static_cast(p); ^ Martin