From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 82093 invoked by alias); 19 Dec 2018 23:28:00 -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 82074 invoked by uid 89); 19 Dec 2018 23:27:58 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-27.6 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 spammy=Stage, combinec, UD:combine.c, combine.c X-HELO: mx0a-001b2d01.pphosted.com Received: from mx0b-001b2d01.pphosted.com (HELO mx0a-001b2d01.pphosted.com) (148.163.158.5) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 19 Dec 2018 23:27:54 +0000 Received: from pps.filterd (m0098420.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.22/8.16.0.22) with SMTP id wBJNEoo1060681 for ; Wed, 19 Dec 2018 18:27:53 -0500 Received: from e35.co.us.ibm.com (e35.co.us.ibm.com [32.97.110.153]) by mx0b-001b2d01.pphosted.com with ESMTP id 2pfybvrv8n-1 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 19 Dec 2018 18:27:52 -0500 Received: from localhost by e35.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 19 Dec 2018 23:27:52 -0000 Received: from b03cxnp07029.gho.boulder.ibm.com (9.17.130.16) by e35.co.us.ibm.com (192.168.1.135) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; (version=TLSv1/SSLv3 cipher=AES256-GCM-SHA384 bits=256/256) Wed, 19 Dec 2018 23:27:49 -0000 Received: from b03ledav006.gho.boulder.ibm.com (b03ledav006.gho.boulder.ibm.com [9.17.130.237]) by b03cxnp07029.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id wBJNRmAI23527516 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 19 Dec 2018 23:27:48 GMT Received: from b03ledav006.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id BB861C6057; Wed, 19 Dec 2018 23:27:48 +0000 (GMT) Received: from b03ledav006.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 83608C6059; Wed, 19 Dec 2018 23:27:38 +0000 (GMT) Received: from ragesh4.local (unknown [9.211.108.112]) by b03ledav006.gho.boulder.ibm.com (Postfix) with ESMTP; Wed, 19 Dec 2018 23:27:38 +0000 (GMT) Subject: Re: [PATCH 2/2] v2: C++: improvements to binary operator diagnostics (PR c++/87504) To: gcc-patches@gcc.gnu.org, dmalcolm@redhat.com References: <1543962908-32306-1-git-send-email-dmalcolm@redhat.com> <1543962908-32306-2-git-send-email-dmalcolm@redhat.com> <1aec1d54-419c-c4d8-2f3e-c34f22ce9559@redhat.com> From: Aaron Sawdey Date: Wed, 19 Dec 2018 23:28:00 -0000 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Thunderbird/60.3.3 MIME-Version: 1.0 In-Reply-To: <1aec1d54-419c-c4d8-2f3e-c34f22ce9559@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit x-cbid: 18121923-0012-0000-0000-000016ED0A8A X-IBM-SpamModules-Scores: X-IBM-SpamModules-Versions: BY=3.00010251; HX=3.00000242; KW=3.00000007; PH=3.00000004; SC=3.00000271; SDB=6.01134199; UDB=6.00589686; IPR=6.00914373; MB=3.00024758; MTD=3.00000008; XFM=3.00000015; UTC=2018-12-19 23:27:50 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 18121923-0013-0000-0000-0000558545ED Message-Id: X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg01428.txt.bz2 Assuming you applied this as svn 267273, it causes bootstrap failure on powerpc64le-unknown-linux-gnu. Stage 2 fails with multiple instances of this error: ../../trunk-base/gcc/c-family/c-pragma.c: In function ‘void handle_pragma_scalar_storage_order(cpp_reader*)’: ../../trunk-base/gcc/c-family/c-pragma.c:417:24: error: self-comparison always evaluates to false [-Werror=tautological-compare] 417 | if (BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN) | ^~ ../../trunk-base/gcc/c-family/c-attribs.c: In function ‘tree_node* handle_scalar_storage_order_attribute(tree_node**, tree, tree, int, bool*)’: ../../trunk-base/gcc/c-family/c-attribs.c:1401:24: error: self-comparison always evaluates to false [-Werror=tautological-compare] 1401 | if (BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN) | ^~ ../../trunk-base/gcc/builtins.c: In function ‘rtx_def* c_readstr(const char*, scalar_int_mode)’: ../../trunk-base/gcc/builtins.c:830:28: error: self-comparison always evaluates to false [-Werror=tautological-compare] 830 | if (BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN | ^~ ../../trunk-base/gcc/combine.c: In function ‘int rtx_equal_for_field_assignment_p(rtx, rtx, bool)’: ../../trunk-base/gcc/combine.c:9668:28: error: self-comparison always evaluates to false [-Werror=tautological-compare] 9668 | if (BYTES_BIG_ENDIAN != WORDS_BIG_ENDIAN) | ^~ Aaron On 12/12/18 2:42 PM, Jason Merrill wrote: > On 12/4/18 5:35 PM, David Malcolm wrote: >> The v1 patch: >>    https://gcc.gnu.org/ml/gcc-patches/2018-11/msg00303.html >> has bitrotten somewhat, so here's v2 of the patch, updated relative >> to r266740. >> >> Blurb from v1 patch follows: >> >> The C frontend is able (where expression locations are available) to print >> problems with binary operators in 3-location form, labelling the types of >> the expressions: >> >>    arg_0 op arg_1 >>    ~~~~~ ^~ ~~~~~ >>      |        | >>      |        arg1 type >>      arg0 type >> >> The C++ frontend currently just shows the combined location: >> >>    arg_0 op arg_1 >>    ~~~~~~^~~~~~~~ >> >> and fails to highlight where the subexpressions are, or their types. >> >> This patch introduces a op_location_t struct for handling the above >> operator-location vs combined-location split, and a new >> class binary_op_rich_location for displaying the above, so that the >> C++ frontend is able to use the more detailed 3-location form for >> type mismatches in binary operators, and for -Wtautological-compare >> (where types are not displayed).  Both forms can be seen in this >> example: >> >> bad-binary-ops.C:69:20: error: no match for 'operator&&' (operand types are >>    's' and 't') >>     69 |   return ns_4::foo && ns_4::inner::bar; >>        |          ~~~~~~~~~ ^~ ~~~~~~~~~~~~~~~~ >>        |                |                   | >>        |                s                   t >> bad-binary-ops.C:69:20: note: candidate: 'operator&&(bool, bool)' >>     69 |   return ns_4::foo && ns_4::inner::bar; >>        |          ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~ >> >> The patch also allows from some uses of macros in >> -Wtautological-compare, where both sides of the comparison have >> been spelled the same way, e.g.: >> >> Wtautological-compare-ranges.c:23:11: warning: self-comparison always >>     evaluates to true [-Wtautological-compare] >>     23 |   if (FOO == FOO); >>        |           ^~ >> >> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu, in >> conjunction with the previous patch. >> >> OK for trunk? >> Dave >> >> gcc/c-family/ChangeLog: >>     PR c++/87504 >>     * c-common.h (warn_tautological_cmp): Convert 1st param from >>     location_t to const op_location_t &. >>     * c-warn.c (find_array_ref_with_const_idx_r): Strip location >>     wrapper when testing for INTEGER_CST. >>     (warn_tautological_bitwise_comparison): Convert 1st param from >>     location_t to const op_location_t &; use it to build a >>     binary_op_rich_location, and use this. >>     (spelled_the_same_p): New function. >>     (warn_tautological_cmp): Convert 1st param from location_t to >>     const op_location_t &.  Warn for macro expansions if >>     spelled_the_same_p.  Use binary_op_rich_location. >> >> gcc/c/ChangeLog: >>     PR c++/87504 >>     * c-typeck.c (class maybe_range_label_for_tree_type_mismatch): >>     Move from here to gcc-rich-location.h and gcc-rich-location.c. >>     (build_binary_op): Use struct op_location_t and >>     class binary_op_rich_location. >> >> gcc/cp/ChangeLog: >>     PR c++/87504 >>     * call.c (op_error): Convert 1st param from location_t to >>     const op_location_t &.  Use binary_op_rich_location for binary >>     ops. >>     (build_conditional_expr_1): Convert 1st param from location_t to >>     const op_location_t &. >>     (build_conditional_expr): Likewise. >>     (build_new_op_1): Likewise. >>     (build_new_op): Likewise. >>     * cp-tree.h (build_conditional_expr): Likewise. >>     (build_new_op): Likewise. >>     (build_x_binary_op): Likewise. >>     (cp_build_binary_op): Likewise. >>     * parser.c (cp_parser_primary_expression): Build a location >>     for id-expression nodes. >>     (cp_parser_binary_expression): Use an op_location_t when >>     calling build_x_binary_op. >>     (cp_parser_operator): Build a location for user-defined literals. >>     * typeck.c (build_x_binary_op): Convert 1st param from location_t >>     to const op_location_t &. >>     (cp_build_binary_op): Likewise.  Use binary_op_rich_location. >> >> gcc/ChangeLog: >>     PR c++/87504 >>     * gcc-rich-location.c >>     (maybe_range_label_for_tree_type_mismatch::get_text): Move here from >>     c/c-typeck.c. >>     (binary_op_rich_location::binary_op_rich_location): New ctor. >>     (binary_op_rich_location::use_operator_loc_p): New function. >>     * gcc-rich-location.h >>     (class maybe_range_label_for_tree_type_mismatch)): Move here from >>     c/c-typeck.c. >>     (struct op_location_t): New forward decl. >>     (class binary_op_rich_location): New class. >>     * tree.h (struct op_location_t): New struct. >> >> gcc/testsuite/ChangeLog: >>     * c-c++-common/Wtautological-compare-ranges.c: New test. >>     * g++.dg/cpp0x/pr51420.C: Add -fdiagnostics-show-caret and update >>     expected output. >>     * g++.dg/diagnostic/bad-binary-ops.C: Update expected output from >>     1-location form to 3-location form, with labelling of ranges with >>     types.  Add examples of id-expression nodes with namespaces. >>     * g++.dg/diagnostic/param-type-mismatch-2.C: Likewise. >> >> This is the 2nd commit message: >> >> FIXME: column and multiline fixes to * g++.dg/cpp0x/pr51420.C >> --- >>   gcc/c-family/c-common.h                            |  3 +- >>   gcc/c-family/c-warn.c                              | 57 +++++++++++--- >>   gcc/c/c-typeck.c                                   | 41 +--------- >>   gcc/cp/call.c                                      | 28 ++++--- >>   gcc/cp/cp-tree.h                                   | 10 ++- >>   gcc/cp/parser.c                                    | 32 ++++++-- >>   gcc/cp/typeck.c                                    | 14 ++-- >>   gcc/gcc-rich-location.c                            | 89 ++++++++++++++++++++++ >>   gcc/gcc-rich-location.h                            | 57 ++++++++++++++ >>   .../c-c++-common/Wtautological-compare-ranges.c    | 42 ++++++++++ >>   gcc/testsuite/g++.dg/cpp0x/pr51420.C               | 10 +++ >>   gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C   | 57 +++++++++++++- >>   .../g++.dg/diagnostic/param-type-mismatch-2.C      |  4 +- >>   gcc/tree.h                                         | 49 ++++++++++++ >>   14 files changed, 417 insertions(+), 76 deletions(-) >>   create mode 100644 gcc/testsuite/c-c++-common/Wtautological-compare-ranges.c >> >> diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h >> index 4187343..0b9ddf6 100644 >> --- a/gcc/c-family/c-common.h >> +++ b/gcc/c-family/c-common.h >> @@ -1268,7 +1268,8 @@ extern void constant_expression_error (tree); >>   extern void overflow_warning (location_t, tree, tree = NULL_TREE); >>   extern void warn_logical_operator (location_t, enum tree_code, tree, >>                      enum tree_code, tree, enum tree_code, tree); >> -extern void warn_tautological_cmp (location_t, enum tree_code, tree, tree); >> +extern void warn_tautological_cmp (const op_location_t &, enum tree_code, >> +                   tree, tree); >>   extern void warn_logical_not_parentheses (location_t, enum tree_code, tree, >>                         tree); >>   extern bool warn_if_unused_value (const_tree, location_t); >> diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c >> index fc7f87c..fce9d84 100644 >> --- a/gcc/c-family/c-warn.c >> +++ b/gcc/c-family/c-warn.c >> @@ -322,7 +322,8 @@ find_array_ref_with_const_idx_r (tree *expr_p, int *, void *) >>       if ((TREE_CODE (expr) == ARRAY_REF >>          || TREE_CODE (expr) == ARRAY_RANGE_REF) >> -      && TREE_CODE (TREE_OPERAND (expr, 1)) == INTEGER_CST) >> +      && (TREE_CODE (tree_strip_any_location_wrapper (TREE_OPERAND (expr, 1))) >> +      == INTEGER_CST)) >>       return integer_type_node; > > I think we want fold_for_warn here.  OK with that change (assuming it passes). > > Jason > -- Aaron Sawdey, Ph.D. acsawdey@linux.vnet.ibm.com 050-2/C113 (507) 253-7520 home: 507/263-0782 IBM Linux Technology Center - PPC Toolchain