This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] v2: C++: improvements to binary operator diagnostics (PR c++/87504)


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)' <built-in>
>>     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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]