From patchwork Mon Jun 23 12:09:23 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 362774 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id E3EF914007C for ; Mon, 23 Jun 2014 22:09:42 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=r2IelzAFvVEXNutn9 xIHPt/hPxZHHTjWno1+ybn1yC4ZT9Gky0RY8zubb5A/oSP0JiAvFnmr0kIfcXI3P Ws9K8R3zqe71ifEPx02LGj2ky08x2cd2fnZgG43A4v68OnCx26dXFMHYH33OzGZU GNaSV0CgsqzTDjiZ4qUj+Qik8w= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=default; bh=I5zFix+R7Y5kG18NkNbFHR4 qdHc=; b=fqUOS6j7zh2y38fWM70pw3OLBZnBSK9ZwFlET7/pnyoVqdFp/+44Rz/ TBj/r942f1SeSgAcrgPgWdf3LU8PBvZ3dDSTeEZIs+2Do9utOmYUA28PJKvhbOWv EBQgibFpBqX94a635VTYdMymX2MNl16qPfChSEbz/28YiZmtmyWE= Received: (qmail 2422 invoked by alias); 23 Jun 2014 12:09:34 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 2412 invoked by uid 89); 23 Jun 2014 12:09:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 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 (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 23 Jun 2014 12:09:31 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s5NC9S8V011009 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 23 Jun 2014 08:09:28 -0400 Received: from redhat.com (ovpn-116-31.ams2.redhat.com [10.36.116.31]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s5NC9OPM012963 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Mon, 23 Jun 2014 08:09:26 -0400 Date: Mon, 23 Jun 2014 14:09:23 +0200 From: Marek Polacek To: Gerald Pfeifer Cc: gcc-patches@gcc.gnu.org, Jason Merrill , "Joseph S. Myers" Subject: Re: [C/C++ PATCH] Add -Wlogical-not-parentheses (PR c/49706) Message-ID: <20140623120923.GJ14420@redhat.com> References: <20140602065052.GE31671@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) On Sun, Jun 22, 2014 at 10:33:57PM +0200, Gerald Pfeifer wrote: > On Mon, 2 Jun 2014, Marek Polacek wrote: > > * c-typeck.c (parser_build_binary_op): Warn when logical not is used > > on the left hand side operand of a comparison. > > This... > > > +/* Warn about logical not used on the left hand side operand of a comparison. > > ...and this... > > > + warning_at (location, OPT_Wlogical_not_parentheses, > > + "logical not is only applied to the left hand side of " > > + "comparison"); > > ...does not appear consistent with the actual warning. > > Why does that warning say "is _ONLY_ applied to the left hand side"? > > Based on the message, I naively assumed that the code should not warn > about > > int same(int a, int b) { > return !a == !b; > } > > alas this is not the case. (Code like this occurs in Wine where > bool types are emulated and !!a or a comparison like above ensure > that those emulated bools are normalized to either 0 or 1.) > > > I understand there is ambiguity in cases like > > return !a == b; > > where the warning would be approriately worded and the programmer > might have intended !(a == b). > > > I do recommend to either omit "only" from the text of the warning > or not warn for cases where ! occurs on both sides of the comparison > (and keep the text as is). I think the latter is better, incidentally, g++ doesn't warn either. The following one liner makes cc1 behave as cc1plus. Thanks for the report. Regtested/bootstrapped on x86_64. Joseph, is this ok? 2014-06-23 Marek Polacek * c-typeck.c (parser_build_binary_op): Don't call warn_logical_not_parentheses if the RHS is TRUTH_NOT_EXPR. * c-c++-common/pr49706-2.c: New test. Marek diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index 63bd65e..0764630 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -3402,7 +3402,8 @@ parser_build_binary_op (location_t location, enum tree_code code, code1, arg1.value, code2, arg2.value); if (warn_logical_not_paren - && code1 == TRUTH_NOT_EXPR) + && code1 == TRUTH_NOT_EXPR + && code2 != TRUTH_NOT_EXPR) warn_logical_not_parentheses (location, code, arg1.value, arg2.value); /* Warn about comparisons against string literals, with the exception diff --git gcc/testsuite/c-c++-common/pr49706-2.c gcc/testsuite/c-c++-common/pr49706-2.c index e69de29..09cc9eb 100644 --- gcc/testsuite/c-c++-common/pr49706-2.c +++ gcc/testsuite/c-c++-common/pr49706-2.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-Wlogical-not-parentheses" } */ + +/* Test that we don't warn if both operands of the comparison + are negated. */ + +#ifndef __cplusplus +#define bool _Bool +#endif + +bool r; + +int +same (int a, int b) +{ + r = !a == !b; + r = !!a == !!b; + r = !!a == !b; + r = !a == !!b; +}