From patchwork Fri Feb 20 00:03:26 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 441798 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 04BDE140146 for ; Fri, 20 Feb 2015 11:03:49 +1100 (AEDT) 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:reply-to:mime-version :content-type; q=dns; s=default; b=UUP/q3M8rpJ1lKuriF85Kei97YKAu wPb0KTXQ741NEAm77ydd8zSCI/QkQ/Ci7N7lTtV1DeY29JsXuNVhTj1eECaOOti9 06mAETB9zL6XxkdRLMDcBVValmCRBv6Sl/EMuXloW8LQ1NP9vcaIuQFszq0BpbpX +jIGciC2tLldDM= 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:reply-to:mime-version :content-type; s=default; bh=kS/4xUHkSJJKpqgQO3jros2XL9Q=; b=MXP mdbs2ILYIN6KC+IDJjg+vdtJdb/RT1Oc1QGAh+DtiHNfH4piIi4hDviNlKmrQCuG F1W4H/vqRkDA5jHwLitR6yyCxgrH3bnApK4auKEkGy5xEwk/DUNlXbZSR6KmQtQT lY3aBamZmCWtDhif3DMkqKKyh9EJNTlbYnbiV2HU= Received: (qmail 1876 invoked by alias); 20 Feb 2015 00:03:42 -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 1867 invoked by uid 89); 20 Feb 2015 00:03:41 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 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; Fri, 20 Feb 2015 00:03:40 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t1K03VJG010321 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 19 Feb 2015 19:03:31 -0500 Received: from tucnak.zalov.cz (ovpn-116-28.ams2.redhat.com [10.36.116.28]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t1K03TYr009579 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Thu, 19 Feb 2015 19:03:31 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.14.9/8.14.9) with ESMTP id t1K03S34030607; Fri, 20 Feb 2015 01:03:28 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.14.9/8.14.9/Submit) id t1K03QDn030606; Fri, 20 Feb 2015 01:03:26 +0100 Date: Fri, 20 Feb 2015 01:03:26 +0100 From: Jakub Jelinek To: "Joseph S. Myers" , Jason Merrill Cc: gcc-patches@gcc.gnu.org Subject: [C/C++ PATCH] -Wlogical-not-parentheses tweaks (PR c/65120) Message-ID: <20150220000326.GT1746@tucnak.redhat.com> Reply-To: Jakub Jelinek MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes Hi! As reported, !!x == y is quite common in the Linux kernel and unlike the !x == y case it usually doesn't mean mistyped !(x == y) or x != y. clang++ apparently doesn't warn about that either, and it doesn't warn even about the case where ! is applied to a bool. As the argument is already folded, it isn't easy to determine those cases always, but I hope the following is sufficient until we switch to late folding. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2015-02-19 Jakub Jelinek PR c/65120 * c-typeck.c (parser_build_binary_op): Don't warn for !!x == y or !b == y where b is _Bool. * parser.c (cp_parser_binary_expression): Don't warn for !!x == y or !b == y where b is bool. * c-c++-common/pr49706.c: Adjust tests for not warning about !!x == y or !b == y where b is boolean, and add some further tests. * c-c++-common/pr62199-2.c: Likewise. Jakub --- gcc/c/c-typeck.c.jj 2015-02-09 22:19:53.000000000 +0100 +++ gcc/c/c-typeck.c 2015-02-19 22:35:38.522781551 +0100 @@ -3460,8 +3460,34 @@ parser_build_binary_op (location_t locat if (warn_logical_not_paren && code1 == TRUTH_NOT_EXPR - && code2 != TRUTH_NOT_EXPR) - warn_logical_not_parentheses (location, code, arg2.value); + && code2 != TRUTH_NOT_EXPR + /* Avoid warning for !!x == y. */ + && (TREE_CODE (arg1.value) != NE_EXPR + || !integer_zerop (TREE_OPERAND (arg1.value, 1)))) + { + /* Avoid warning for !b == y where b has _Bool type. */ + tree t = integer_zero_node; + if (TREE_CODE (arg1.value) == EQ_EXPR + && integer_zerop (TREE_OPERAND (arg1.value, 1)) + && TREE_TYPE (TREE_OPERAND (arg1.value, 0)) == integer_type_node) + { + t = TREE_OPERAND (arg1.value, 0); + do + { + if (TREE_TYPE (t) != integer_type_node) + break; + if (TREE_CODE (t) == C_MAYBE_CONST_EXPR) + t = C_MAYBE_CONST_EXPR_EXPR (t); + else if (CONVERT_EXPR_P (t)) + t = TREE_OPERAND (t, 0); + else + break; + } + while (1); + } + if (TREE_CODE (TREE_TYPE (t)) != BOOLEAN_TYPE) + warn_logical_not_parentheses (location, code, arg2.value); + } /* Warn about comparisons against string literals, with the exception of testing for equality or inequality of a string literal with NULL. */ --- gcc/cp/parser.c.jj 2015-02-14 00:19:49.000000000 +0100 +++ gcc/cp/parser.c 2015-02-19 22:04:19.059604707 +0100 @@ -8270,7 +8270,20 @@ cp_parser_binary_expression (cp_parser* c_inhibit_evaluation_warnings -= current.lhs == truthvalue_true_node; if (warn_logical_not_paren - && current.lhs_type == TRUTH_NOT_EXPR) + && current.lhs_type == TRUTH_NOT_EXPR + /* Avoid warning for !!x == y. */ + && (TREE_CODE (current.lhs) != NE_EXPR + || !integer_zerop (TREE_OPERAND (current.lhs, 1))) + && (TREE_CODE (current.lhs) != TRUTH_NOT_EXPR + || (TREE_CODE (TREE_OPERAND (current.lhs, 0)) != TRUTH_NOT_EXPR + /* Avoid warning for !b == y where b is boolean. */ + && (TREE_TYPE (TREE_OPERAND (current.lhs, 0)) == NULL_TREE + || (TREE_CODE (TREE_TYPE (TREE_OPERAND (current.lhs, 0))) + != BOOLEAN_TYPE)))) + /* Avoid warning for !!b == y where b is boolean. */ + && (!DECL_P (current.lhs) + || TREE_TYPE (current.lhs) == NULL_TREE + || TREE_CODE (TREE_TYPE (current.lhs)) != BOOLEAN_TYPE)) warn_logical_not_parentheses (current.loc, current.tree_type, rhs); overload = NULL; --- gcc/testsuite/c-c++-common/pr49706.c.jj 2014-06-06 09:19:20.000000000 +0200 +++ gcc/testsuite/c-c++-common/pr49706.c 2015-02-19 20:53:27.466594656 +0100 @@ -12,10 +12,13 @@ extern bool foo_b (void); extern int foo_i (void); #ifdef __cplusplus -template bool f1(T t, U u) { return (!t == u); } /* { dg-warning "logical not is only applied to the left hand side of comparison" "" { target c++ } 15 } */ -template bool f2(T t, U u) { return ((!t) == u); } -template bool f3(T t, U u) { return (!g(t) == u); } /* { dg-warning "logical not is only applied to the left hand side of comparison" "" { target c++ } 17 } */ -template bool f4(T t, U u) { return ((!g(t)) == u); } +template bool tfn1(T t, U u) { return (!t == u); } /* { dg-warning "logical not is only applied to the left hand side of comparison" "" { target c++ } 15 } */ +template bool tfn2(T t, U u) { return ((!t) == u); } +template bool tfn3(T t, U u) { return (!g(t) == u); } /* { dg-warning "logical not is only applied to the left hand side of comparison" "" { target c++ } 17 } */ +template bool tfn4(T t, U u) { return ((!g(t)) == u); } +template bool tfn5(T t, U u) { return (!!t == u); } /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ +template bool tfn6(T t, U u) { return (!!g(t) == u); } /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ +template bool tfn7(int i1, int i2) { return (!i1 == i2); } /* { dg-warning "logical not is only applied to the left hand side of comparison" "" { target c++ } 21 } */ #endif void @@ -58,23 +61,42 @@ fn1 (int i1, int i2, bool b1, bool b2) b = !b1 <= b2; b = !b1 >= b2; + b = !b1 == i2; + b = !b1 != i2; + b = !b1 < i2; + b = !b1 > i2; + b = !b1 <= i2; + b = !b1 >= i2; + b = !foo_i () == i1; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ b = (!foo_i ()) == i1; b = !foo_b () == b1; - b = !!i1 == i2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - b = !!i1 != i2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - b = !!i1 < i2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - b = !!i1 > i2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - b = !!i1 <= i2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - b = !!i1 >= i2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - b = !!foo_i () == i1; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 == i2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 != i2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 < i2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 > i2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 <= i2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 >= i2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!foo_i () == i1; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + + b = !!b1 == i2; + b = !!b1 != i2; + b = !!b1 < i2; + b = !!b1 > i2; + b = !!b1 <= i2; + b = !!b1 >= i2; /* Be careful here. */ b = (i1 == 0) != 0; b = (i1 == 0) == 0; b = (i1 != 0) != 0; b = (i1 != 0) == 0; + + b = !5 == 4; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = !!5 == 4; /* { dg-bogus "logical not is only applied to the left hand side of comparison" "" { xfail *-*-* } } */ + b = !1 == 1; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = !!1 == 1; /* { dg-bogus "logical not is only applied to the left hand side of comparison" "" { xfail *-*-* } } */ } void @@ -100,3 +122,44 @@ fn2 (enum E e) b = (!foo_e ()) == A; b = (!foo_e ()) == foo_e (); } + +void +fn3 (int i1, float f2) +{ + b = !i1 == f2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = !i1 != f2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = !i1 < f2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = !i1 > f2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = !i1 <= f2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = !i1 >= f2; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + + b = i1 == f2; + b = i1 != f2; + b = i1 < f2; + b = i1 > f2; + b = i1 <= f2; + b = i1 >= f2; + + /* Parens suppress the warning. */ + b = (!i1) == f2; + b = (!i1) != f2; + b = (!i1) < f2; + b = (!i1) > f2; + b = (!i1) <= f2; + b = (!i1) >= f2; + + /* ...but not these parens. */ + b = (!i1 == f2); /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = (!i1 != f2); /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = (!i1 < f2); /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = (!i1 > f2); /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = (!i1 <= f2); /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + b = (!i1 >= f2); /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + + b = !!i1 == f2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 != f2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 < f2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 > f2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 <= f2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + b = !!i1 >= f2; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ +} --- gcc/testsuite/c-c++-common/pr62199-2.c.jj 2014-09-01 09:43:39.000000000 +0200 +++ gcc/testsuite/c-c++-common/pr62199-2.c 2015-02-20 00:57:07.362564155 +0100 @@ -11,10 +11,10 @@ bool r; void foo (bool b) { - r = !b == 1; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - r = !b != 1; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - r = !b > 1; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - r = !b >= 1; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - r = !b < 1; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ - r = !b <= 1; /* { dg-warning "logical not is only applied to the left hand side of comparison" } */ + r = !b == 1; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + r = !b != 1; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + r = !b > 1; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + r = !b >= 1; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + r = !b < 1; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ + r = !b <= 1; /* { dg-bogus "logical not is only applied to the left hand side of comparison" } */ }