This is the mail archive of the 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)

On 12/4/18 5:35 PM, David Malcolm wrote:
The v1 patch:
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

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?

	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.

	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.

	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
	(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.

	PR c++/87504
	* gcc-rich-location.c
	(maybe_range_label_for_tree_type_mismatch::get_text): Move here from
	(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
	(struct op_location_t): New forward decl.
	(class binary_op_rich_location): New class.
	* tree.h (struct op_location_t): New struct.

	* 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,
  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).


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