From patchwork Wed Aug 24 18:15:49 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 662439 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 3sKFqV20FVz9syB for ; Thu, 25 Aug 2016 04:16:04 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=otI2nrH4; dkim-atps=neutral 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:content-transfer-encoding:in-reply-to; q=dns; s= default; b=YcoNV3j150mzSqVRY8aToNsk3Fo4ZDttP1BrGrVz/mcVQFWi+TmGX n2Ewa2Ap7XrfCinmTwNIrD89EkxT5dDLNcZ9TnbXb5o0nH7QYnganLRcABwPyS0G mjSgRVhZITyCIbY/ebAWe/giicAviUo6Z6A7Rs0hPNkDy/gxdX1IxU= 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:content-transfer-encoding:in-reply-to; s=default; bh=c9nQ3TJthHzE5dlPsMMpuMKEmfE=; b=otI2nrH4zs1B1ZlQ4sa6o3fV6HmQ Z67PVSUv53gzUVUC+uuJBLE5patcrfuEvY2B3UT6yJ1LEQ7AP35Kme7/k5pXxWT0 s4R13vfVxLbir1vAlS+pVx5jkm7gf1tjJE6ykil4aO+FsQvwlRbAW3dH3o9m1eq4 d9Xa72bUIBjbGrQ= Received: (qmail 9782 invoked by alias); 24 Aug 2016 18:15:56 -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 9741 invoked by uid 89); 24 Aug 2016 18:15:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=15pm, bbb 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; Wed, 24 Aug 2016 18:15:54 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 01A2761E5F for ; Wed, 24 Aug 2016 18:15:53 +0000 (UTC) Received: from redhat.com (ovpn-204-109.brq.redhat.com [10.40.204.109]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u7OIFnct013421 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 24 Aug 2016 14:15:52 -0400 Date: Wed, 24 Aug 2016 20:15:49 +0200 From: Marek Polacek To: David Malcolm Cc: GCC Patches Subject: Re: Add fixit hint for -Wlogical-not-parentheses Message-ID: <20160824181549.GL11131@redhat.com> References: <20160824174346.GK11131@redhat.com> <1472061435.8216.109.camel@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1472061435.8216.109.camel@redhat.com> User-Agent: Mutt/1.6.2 (2016-07-01) On Wed, Aug 24, 2016 at 01:57:15PM -0400, David Malcolm wrote: > On Wed, 2016-08-24 at 19:43 +0200, Marek Polacek wrote: > > This patch adds a fixit hint to -Wlogical-not-parentheses. When the > > LHS > > has a location, it prints: > > > > z.c: In function ‘int foo(int, int)’: > > z.c:12:11: warning: logical not is only applied to the left hand side > > of comparison [-Wlogical-not-parentheses] > > r += !a == b; > > ^~ > > z.c:12:8: note: add parentheses around left hand side expression to > > silence this warning > > r += !a == b; > > ^~ > > ( ) > > > > Bootstrapped/regtested on x86_64-linux and ppc64le-redhat-linux, ok > > for trunk? > > Thanks for writing new fix-it hints. > > Can you split this up into two fix-its, one for each parenthesis, at > the appropriate locations? A single rich_location (and thus > diagnostic) can contain up to 3 fix-it hints at the moment. My hope > is that an IDE could, in theory, apply them; right now, the fixit is > effectively saying to make this change: > > - r += !a == b; > + r += ( )!a == b; > > whereas presumably it should be making this change: > > - r += !a == b; > + r += (!a) == b; True. > You should be able to access the source end-point of the expression > via: > get_range_from_loc (line_table, loc).m_finish I see, thanks. So like in the below? > Also, when writing testcases that involve -fdiagnostics-show-caret, > please use identifier names that are longer than one character: this > makes it easier to verify that we're using the correct ranges. Done. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2016-08-24 Marek Polacek * c-common.c (warn_logical_not_parentheses): Print fixit hints. * c-common.h (warn_logical_not_parentheses): Update declaration. * c-typeck.c (parser_build_binary_op): Pass LHS to warn_logical_not_parentheses. * parser.c (cp_parser_binary_expression): Pass LHS to warn_logical_not_parentheses. * c-c++-common/Wlogical-not-parentheses-2.c: New test. Marek diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c index 3feb910..1059466 100644 --- gcc/c-family/c-common.c +++ gcc/c-family/c-common.c @@ -1485,7 +1485,7 @@ warn_tautological_cmp (location_t loc, enum tree_code code, tree lhs, tree rhs) void warn_logical_not_parentheses (location_t location, enum tree_code code, - tree rhs) + tree lhs, tree rhs) { if (TREE_CODE_CLASS (code) != tcc_comparison || TREE_TYPE (rhs) == NULL_TREE @@ -1498,9 +1498,18 @@ warn_logical_not_parentheses (location_t location, enum tree_code code, && integer_zerop (rhs)) return; - warning_at (location, OPT_Wlogical_not_parentheses, - "logical not is only applied to the left hand side of " - "comparison"); + if (warning_at (location, OPT_Wlogical_not_parentheses, + "logical not is only applied to the left hand side of " + "comparison") + && EXPR_HAS_LOCATION (lhs)) + { + location_t lhs_loc = EXPR_LOCATION (lhs); + rich_location richloc (line_table, lhs_loc); + richloc.add_fixit_insert (lhs_loc, "("); + richloc.add_fixit_insert (get_finish (lhs_loc), ")"); + inform_at_rich_loc (&richloc, "add parentheses around left hand side " + "expression to silence this warning"); + } } /* Warn if EXP contains any computations whose results are not used. diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h index bc22baa..42ce969 100644 --- gcc/c-family/c-common.h +++ gcc/c-family/c-common.h @@ -847,7 +847,8 @@ extern void overflow_warning (location_t, tree); extern bool warn_if_unused_value (const_tree, location_t); extern void warn_logical_operator (location_t, enum tree_code, tree, enum tree_code, tree, enum tree_code, tree); -extern void warn_logical_not_parentheses (location_t, enum tree_code, tree); +extern void warn_logical_not_parentheses (location_t, enum tree_code, tree, + tree); extern void warn_tautological_cmp (location_t, enum tree_code, tree, tree); extern void check_main_parameter_types (tree decl); extern bool c_determine_visibility (tree); diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index bc8728a..2f8d611 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -3696,7 +3696,7 @@ parser_build_binary_op (location_t location, enum tree_code code, while (1); } if (TREE_CODE (TREE_TYPE (t)) != BOOLEAN_TYPE) - warn_logical_not_parentheses (location, code, arg2.value); + warn_logical_not_parentheses (location, code, arg1.value, arg2.value); } /* Warn about comparisons against string literals, with the exception diff --git gcc/cp/parser.c gcc/cp/parser.c index 690e928..d54cf8a 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -8922,7 +8922,7 @@ cp_parser_binary_expression (cp_parser* parser, bool cast_p, || TREE_TYPE (current.lhs) == NULL_TREE || TREE_CODE (TREE_TYPE (current.lhs)) != BOOLEAN_TYPE)) warn_logical_not_parentheses (current.loc, current.tree_type, - maybe_constant_value (rhs)); + current.lhs, maybe_constant_value (rhs)); overload = NULL; diff --git gcc/testsuite/c-c++-common/Wlogical-not-parentheses-2.c gcc/testsuite/c-c++-common/Wlogical-not-parentheses-2.c index e69de29..c70adc5 100644 --- gcc/testsuite/c-c++-common/Wlogical-not-parentheses-2.c +++ gcc/testsuite/c-c++-common/Wlogical-not-parentheses-2.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-Wlogical-not-parentheses -fdiagnostics-show-caret" } */ + + /* Test fixit hints. */ + +int +foo (int aaa, int bbb) +{ + int r = 0; + r += (!aaa) == bbb; + r += !aaa == bbb; /* { dg-warning "logical not is only applied" } */ +/* { dg-begin-multiline-output "" } + r += !aaa == bbb; + ^~ + r += !aaa == bbb; + ^~~~ + ( ) + { dg-end-multiline-output "" } */ + return r; +}