From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 86542 invoked by alias); 12 Dec 2018 20:43:02 -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 86522 invoked by uid 89); 12 Dec 2018 20:43:01 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_SHORT,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=cfamily, c-family, array_ref X-HELO: mail-qt1-f171.google.com Received: from mail-qt1-f171.google.com (HELO mail-qt1-f171.google.com) (209.85.160.171) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Dec 2018 20:42:58 +0000 Received: by mail-qt1-f171.google.com with SMTP id d18so22078877qto.8 for ; Wed, 12 Dec 2018 12:42:58 -0800 (PST) Return-Path: Received: from [192.168.1.132] (209-6-216-142.s141.c3-0.smr-cbr1.sbo-smr.ma.cable.rcncustomer.com. [209.6.216.142]) by smtp.gmail.com with ESMTPSA id b75sm6076682qkh.4.2018.12.12.12.42.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Dec 2018 12:42:55 -0800 (PST) Subject: Re: [PATCH 2/2] v2: C++: improvements to binary operator diagnostics (PR c++/87504) To: David Malcolm Cc: Jeff Law , gcc-patches@gcc.gnu.org References: <1543962908-32306-1-git-send-email-dmalcolm@redhat.com> <1543962908-32306-2-git-send-email-dmalcolm@redhat.com> From: Jason Merrill Message-ID: <1aec1d54-419c-c4d8-2f3e-c34f22ce9559@redhat.com> Date: Wed, 12 Dec 2018 20:43:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 In-Reply-To: <1543962908-32306-2-git-send-email-dmalcolm@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg00869.txt.bz2 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