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]

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


Ping re this patch:
  "[PATCH 2/2] v2: C++: improvements to binary operator diagnostics (PR
c++/87504)"
     https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00236.html

(...which is dependent on:
  "[PATCH 1/2] v3: C++: more location wrapper nodes (PR c++/43064, PR
c++/43486)"
     https://gcc.gnu.org/ml/gcc-patches/2018-12/msg00500.html )

Thanks
Dave

On Tue, 2018-12-04 at 17:35 -0500, 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;
>  
>    return NULL_TREE;
> @@ -334,7 +335,7 @@ find_array_ref_with_const_idx_r (tree *expr_p,
> int *, void *)
>     of this comparison.  */
>  
>  static void
> -warn_tautological_bitwise_comparison (location_t loc, tree_code
> code,
> +warn_tautological_bitwise_comparison (const op_location_t &loc,
> tree_code code,
>  				      tree lhs, tree rhs)
>  {
>    if (code != EQ_EXPR && code != NE_EXPR)
> @@ -389,29 +390,64 @@ warn_tautological_bitwise_comparison
> (location_t loc, tree_code code,
>    if (res == cstw)
>      return;
>  
> +  binary_op_rich_location richloc (loc, lhs, rhs, false);
>    if (code == EQ_EXPR)
> -    warning_at (loc, OPT_Wtautological_compare,
> +    warning_at (&richloc, OPT_Wtautological_compare,
>  		"bitwise comparison always evaluates to false");
>    else
> -    warning_at (loc, OPT_Wtautological_compare,
> +    warning_at (&richloc, OPT_Wtautological_compare,
>  		"bitwise comparison always evaluates to true");
>  }
>  
> +/* Given LOC_A and LOC_B from macro expansions, return true if
> +   they are "spelled the same" i.e. if they are both directly from
> +   expansion of the same non-function-like macro.  */
> +
> +static bool
> +spelled_the_same_p (location_t loc_a, location_t loc_b)
> +{
> +  gcc_assert (from_macro_expansion_at (loc_a));
> +  gcc_assert (from_macro_expansion_at (loc_b));
> +
> +  const line_map_macro *map_a
> +    = linemap_check_macro (linemap_lookup (line_table, loc_a));
> +
> +  const line_map_macro *map_b
> +    = linemap_check_macro (linemap_lookup (line_table, loc_b));
> +
> +  if (map_a->macro == map_b->macro)
> +    if (!cpp_fun_like_macro_p (map_a->macro))
> +      return true;
> +
> +  return false;
> +}
> +
>  /* Warn if a self-comparison always evaluates to true or false.  LOC
>     is the location of the comparison with code CODE, LHS and RHS are
>     operands of the comparison.  */
>  
>  void
> -warn_tautological_cmp (location_t loc, enum tree_code code, tree
> lhs, tree rhs)
> +warn_tautological_cmp (const op_location_t &loc, enum tree_code
> code,
> +		       tree lhs, tree rhs)
>  {
>    if (TREE_CODE_CLASS (code) != tcc_comparison)
>      return;
>  
>    /* Don't warn for various macro expansions.  */
> -  if (from_macro_expansion_at (loc)
> -      || from_macro_expansion_at (EXPR_LOCATION (lhs))
> -      || from_macro_expansion_at (EXPR_LOCATION (rhs)))
> +  if (from_macro_expansion_at (loc))
>      return;
> +  bool lhs_in_macro = from_macro_expansion_at (EXPR_LOCATION (lhs));
> +  bool rhs_in_macro = from_macro_expansion_at (EXPR_LOCATION (rhs));
> +  if (lhs_in_macro || rhs_in_macro)
> +    {
> +      /* Don't warn if exactly one is from a macro.  */
> +      if (!(lhs_in_macro && rhs_in_macro))
> +	return;
> +
> +      /* If both are in a macro, only warn if they're spelled the
> same.  */
> +      if (!spelled_the_same_p (EXPR_LOCATION (lhs), EXPR_LOCATION
> (rhs)))
> +	return;
> +    }
>  
>    warn_tautological_bitwise_comparison (loc, code, lhs, rhs);
>  
> @@ -446,11 +482,12 @@ warn_tautological_cmp (location_t loc, enum
> tree_code code, tree lhs, tree rhs)
>        const bool always_true = (code == EQ_EXPR || code == LE_EXPR
>  				|| code == GE_EXPR || code ==
> UNLE_EXPR
>  				|| code == UNGE_EXPR || code ==
> UNEQ_EXPR);
> +      binary_op_rich_location richloc (loc, lhs, rhs, false);
>        if (always_true)
> -	warning_at (loc, OPT_Wtautological_compare,
> +	warning_at (&richloc, OPT_Wtautological_compare,
>  		    "self-comparison always evaluates to true");
>        else
> -	warning_at (loc, OPT_Wtautological_compare,
> +	warning_at (&richloc, OPT_Wtautological_compare,
>  		    "self-comparison always evaluates to false");
>      }
>  }
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 8fbecfc..33aad1c 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -11310,38 +11310,6 @@ build_vec_cmp (tree_code code, tree type,
>    return build3 (VEC_COND_EXPR, type, cmp, minus_one_vec, zero_vec);
>  }
>  
> -/* Subclass of range_label for labelling the type of EXPR when
> reporting
> -   a type mismatch between EXPR and OTHER_EXPR.
> -   Either or both of EXPR and OTHER_EXPR could be NULL.  */
> -
> -class maybe_range_label_for_tree_type_mismatch : public range_label
> -{
> - public:
> -  maybe_range_label_for_tree_type_mismatch (tree expr, tree
> other_expr)
> -  : m_expr (expr), m_other_expr (other_expr)
> -  {
> -  }
> -
> -  label_text get_text (unsigned range_idx) const FINAL OVERRIDE
> -  {
> -    if (m_expr == NULL_TREE
> -	|| !EXPR_P (m_expr))
> -      return label_text (NULL, false);
> -    tree expr_type = TREE_TYPE (m_expr);
> -
> -    tree other_type = NULL_TREE;
> -    if (m_other_expr && EXPR_P (m_other_expr))
> -      other_type = TREE_TYPE (m_other_expr);
> -
> -   range_label_for_type_mismatch inner (expr_type, other_type);
> -   return inner.get_text (range_idx);
> -  }
> -
> - private:
> -  tree m_expr;
> -  tree m_other_expr;
> -};
> -
>  /* Build a binary-operation expression without default conversions.
>     CODE is the kind of expression to build.
>     LOCATION is the operator's location.
> @@ -12472,12 +12440,9 @@ build_binary_op (location_t location, enum
> tree_code code,
>  
>    if (!result_type)
>      {
> -      gcc_rich_location richloc (location);
> -      maybe_range_label_for_tree_type_mismatch
> -	label_for_op0 (orig_op0, orig_op1),
> -	label_for_op1 (orig_op1, orig_op0);
> -      richloc.maybe_add_expr (orig_op0, &label_for_op0);
> -      richloc.maybe_add_expr (orig_op1, &label_for_op1);
> +      /* Favor showing any expression locations that are available.
> */
> +      op_location_t oploc (location, UNKNOWN_LOCATION);
> +      binary_op_rich_location richloc (oploc, orig_op0, orig_op1,
> true);
>        binary_op_error (&richloc, code, TREE_TYPE (op0), TREE_TYPE
> (op1));
>        return error_mark_node;
>      }
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 6dd8744..ca8799d 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -166,8 +166,8 @@ static tree build_over_call (struct z_candidate
> *, int, tsubst_flags_t);
>  		     /*c_cast_p=*/false, (COMPLAIN))
>  static tree convert_like_real (conversion *, tree, tree, int, bool,
>  			       bool, tsubst_flags_t);
> -static void op_error (location_t, enum tree_code, enum tree_code,
> tree,
> -		      tree, tree, bool);
> +static void op_error (const op_location_t &, enum tree_code, enum
> tree_code,
> +		      tree, tree, tree, bool);
>  static struct z_candidate *build_user_type_conversion_1 (tree, tree,
> int,
>  							 tsubst_flag
> s_t);
>  static void print_z_candidate (location_t, const char *, struct
> z_candidate *);
> @@ -4713,7 +4713,8 @@ op_error_string (const char *errmsg, int
> ntypes, bool match)
>  }
>  
>  static void
> -op_error (location_t loc, enum tree_code code, enum tree_code code2,
> +op_error (const op_location_t &loc,
> +	  enum tree_code code, enum tree_code code2,
>  	  tree arg1, tree arg2, tree arg3, bool match)
>  {
>    bool assop = code == MODIFY_EXPR;
> @@ -4767,8 +4768,12 @@ op_error (location_t loc, enum tree_code code,
> enum tree_code code2,
>      default:
>        if (arg2)
>  	if (flag_diagnostics_show_caret)
> -	  error_at (loc, op_error_string (G_("%<operator%s%>"), 2,
> match),
> -		    opname, TREE_TYPE (arg1), TREE_TYPE (arg2));
> +	  {
> +	    binary_op_rich_location richloc (loc, arg1, arg2, true);
> +	    error_at (&richloc,
> +		      op_error_string (G_("%<operator%s%>"), 2,
> match),
> +		      opname, TREE_TYPE (arg1), TREE_TYPE (arg2));
> +	  }
>  	else
>  	  error_at (loc, op_error_string (G_("%<operator%s%> in %<%E
> %s %E%>"),
>  					  2, match),
> @@ -4867,7 +4872,8 @@ conditional_conversion (tree e1, tree e2,
> tsubst_flags_t complain)
>     arguments to the conditional expression.  */
>  
>  static tree
> -build_conditional_expr_1 (location_t loc, tree arg1, tree arg2, tree
> arg3,
> +build_conditional_expr_1 (const op_location_t &loc,
> +			  tree arg1, tree arg2, tree arg3,
>                            tsubst_flags_t complain)
>  {
>    tree arg2_type;
> @@ -5461,7 +5467,8 @@ build_conditional_expr_1 (location_t loc, tree
> arg1, tree arg2, tree arg3,
>  /* Wrapper for above.  */
>  
>  tree
> -build_conditional_expr (location_t loc, tree arg1, tree arg2, tree
> arg3,
> +build_conditional_expr (const op_location_t &loc,
> +			tree arg1, tree arg2, tree arg3,
>                          tsubst_flags_t complain)
>  {
>    tree ret;
> @@ -5650,8 +5657,9 @@ op_is_ordered (tree_code code)
>  }
>  
>  static tree
> -build_new_op_1 (location_t loc, enum tree_code code, int flags, tree
> arg1,
> -		tree arg2, tree arg3, tree *overload, tsubst_flags_t
> complain)
> +build_new_op_1 (const op_location_t &loc, enum tree_code code, int
> flags,
> +		tree arg1, tree arg2, tree arg3, tree *overload,
> +		tsubst_flags_t complain)
>  {
>    struct z_candidate *candidates = 0, *cand;
>    vec<tree, va_gc> *arglist;
> @@ -6130,7 +6138,7 @@ build_new_op_1 (location_t loc, enum tree_code
> code, int flags, tree arg1,
>  /* Wrapper for above.  */
>  
>  tree
> -build_new_op (location_t loc, enum tree_code code, int flags,
> +build_new_op (const op_location_t &loc, enum tree_code code, int
> flags,
>  	      tree arg1, tree arg2, tree arg3,
>  	      tree *overload, tsubst_flags_t complain)
>  {
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 111a123..e80334a 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6097,7 +6097,8 @@ extern int raw_dump_id;
>  extern bool check_dtor_name			(tree, tree);
>  int magic_varargs_p				(tree);
>  
> -extern tree build_conditional_expr		(location_t, tree,
> tree, tree, 
> +extern tree build_conditional_expr		(const
> op_location_t &,
> +						 tree, tree, tree,
>                                                   tsubst_flags_t);
>  extern tree build_addr_func			(tree,
> tsubst_flags_t);
>  extern void set_flags_from_callee		(tree);
> @@ -6122,7 +6123,8 @@ extern tree build_new_method_call		
> (tree, tree,
>  extern tree build_special_member_call		(tree, tree,
>  						 vec<tree, va_gc>
> **,
>  						 tree, int,
> tsubst_flags_t);
> -extern tree build_new_op			(location_t, enum
> tree_code,
> +extern tree build_new_op			(const op_location_t
> &,
> +						 enum tree_code,
>  						 int, tree, tree,
> tree, tree *,
>  						 tsubst_flags_t);
>  extern tree build_op_call			(tree, vec<tree,
> va_gc> **,
> @@ -7338,7 +7340,7 @@ extern tree
> cp_build_function_call_nary         (tree, tsubst_flags_t, ...)
>  						ATTRIBUTE_SENTINEL;
>  extern tree cp_build_function_call_vec		(tree,
> vec<tree, va_gc> **,
>  						 tsubst_flags_t);
> -extern tree build_x_binary_op			(location_t,
> +extern tree build_x_binary_op			(const
> op_location_t &,
>  						 enum tree_code,
> tree,
>  						 enum tree_code,
> tree,
>  						 enum tree_code,
> tree *,
> @@ -7405,7 +7407,7 @@ extern tree composite_pointer_type		
> (tree, tree, tree, tree,
>  extern tree merge_types				(tree, tree);
>  extern tree strip_array_domain			(tree);
>  extern tree check_return_expr			(tree, bool *);
> -extern tree cp_build_binary_op                  (location_t,
> +extern tree cp_build_binary_op                  (const op_location_t
> &,
>  						 enum tree_code,
> tree, tree,
>  						 tsubst_flags_t);
>  extern tree build_x_vec_perm_expr               (location_t,
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 4acc79d..321ff39 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -5710,8 +5710,21 @@ cp_parser_primary_expression (cp_parser
> *parser,
>  		 id_expression.get_location ()));
>  	if (error_msg)
>  	  cp_parser_error (parser, error_msg);
> -	decl.set_location (id_expression.get_location ());
> -	decl.set_range (id_expr_token->location,
> id_expression.get_finish ());
> +	/* Build a location for an id-expression of the form:
> +	     ::ns::id
> +             ~~~~~~^~
> +	  or:
> +	     id
> +	     ^~
> +	   i.e. from the start of the first token to the end of the
> final
> +	   token, with the caret at the start of the unqualified-
> id.  */
> +	location_t caret_loc = get_pure_location
> (id_expression.get_location ());
> +	location_t start_loc = get_start (id_expr_token->location);
> +	location_t finish_loc = get_finish
> (id_expression.get_location ());
> +	location_t combined_loc
> +	  = make_location (caret_loc, start_loc, finish_loc);
> +
> +	decl.set_location (combined_loc);
>  	return decl;
>        }
>  
> @@ -9547,7 +9560,8 @@ cp_parser_binary_expression (cp_parser* parser,
> bool cast_p,
>  	}
>        else
>          {
> -          current.lhs = build_x_binary_op (combined_loc,
> current.tree_type,
> +	  op_location_t op_loc (current.loc, combined_loc);
> +	  current.lhs = build_x_binary_op (op_loc,
> current.tree_type,
>                                             current.lhs,
> current.lhs_type,
>                                             rhs, rhs_type, &overload,
>                                             complain_flags
> (decltype_p));
> @@ -15382,8 +15396,16 @@ cp_parser_operator (cp_parser* parser,
> location_t start_loc)
>  	    const char *name = IDENTIFIER_POINTER (id);
>  	    id = cp_literal_operator_id (name);
>  	  }
> -	start_loc = make_location (start_loc, start_loc, get_finish
> (end_loc));
> -	return cp_expr (id, start_loc);
> +	/* Generate a location of the form:
> +	     "" _suffix_identifier
> +	     ^~~~~~~~~~~~~~~~~~~~~
> +	   with caret == start at the start token, finish at the end
> of the
> +	   suffix identifier.  */
> +	location_t finish_loc
> +	  = get_finish (cp_lexer_previous_token (parser->lexer)-
> >location);
> +	location_t combined_loc
> +	  = make_location (start_loc, start_loc, finish_loc);
> +	return cp_expr (id, combined_loc);
>        }
>  
>      default:
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index 820cfc5..a2f75c8 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -4114,7 +4114,7 @@ convert_arguments (tree typelist, vec<tree,
> va_gc> **values, tree fndecl,
>     ARG2_CODE as ERROR_MARK.  */
>  
>  tree
> -build_x_binary_op (location_t loc, enum tree_code code, tree arg1,
> +build_x_binary_op (const op_location_t &loc, enum tree_code code,
> tree arg1,
>  		   enum tree_code arg1_code, tree arg2,
>  		   enum tree_code arg2_code, tree *overload_p,
>  		   tsubst_flags_t complain)
> @@ -4303,7 +4303,7 @@ warn_for_null_address (location_t location,
> tree op, tsubst_flags_t complain)
>     multiple inheritance, and deal with pointer to member
> functions.  */
>  
>  tree
> -cp_build_binary_op (location_t location,
> +cp_build_binary_op (const op_location_t &location,
>  		    enum tree_code code, tree orig_op0, tree
> orig_op1,
>  		    tsubst_flags_t complain)
>  {
> @@ -5300,9 +5300,13 @@ cp_build_binary_op (location_t location,
>    if (!result_type)
>      {
>        if (complain & tf_error)
> -	error_at (location,
> -		  "invalid operands of types %qT and %qT to binary
> %qO",
> -		  TREE_TYPE (orig_op0), TREE_TYPE (orig_op1), code);
> +	{
> +	  binary_op_rich_location richloc (location,
> +					   orig_op0, orig_op1,
> true);
> +	  error_at (&richloc,
> +		    "invalid operands of types %qT and %qT to binary
> %qO",
> +		    TREE_TYPE (orig_op0), TREE_TYPE (orig_op1),
> code);
> +	}
>        return error_mark_node;
>      }
>  
> diff --git a/gcc/gcc-rich-location.c b/gcc/gcc-rich-location.c
> index 81beb61..25a604f 100644
> --- a/gcc/gcc-rich-location.c
> +++ b/gcc/gcc-rich-location.c
> @@ -182,3 +182,92 @@ gcc_rich_location::add_fixit_insert_formatted
> (const char *content,
>    else
>      add_fixit_insert_before (insertion_point, content);
>  }
> +
> +/* Implementation of range_label::get_text for
> +   maybe_range_label_for_tree_type_mismatch.
> +
> +   If both expressions are non-NULL, then generate text describing
> +   the first expression's type (using the other expression's type
> +   for comparison, analogous to %H and %I in the C++ frontend, but
> +   on expressions rather than types).  */
> +
> +label_text
> +maybe_range_label_for_tree_type_mismatch::get_text (unsigned
> range_idx) const
> +{
> +  if (m_expr == NULL_TREE
> +      || !EXPR_P (m_expr))
> +    return label_text (NULL, false);
> +  tree expr_type = TREE_TYPE (m_expr);
> +
> +  tree other_type = NULL_TREE;
> +  if (m_other_expr && EXPR_P (m_other_expr))
> +    other_type = TREE_TYPE (m_other_expr);
> +
> +  range_label_for_type_mismatch inner (expr_type, other_type);
> +  return inner.get_text (range_idx);
> +}
> +
> +/* binary_op_rich_location's ctor.
> +
> +   If use_operator_loc_p (LOC, ARG0, ARG1), then attempt to make a
> 3-location
> +   rich_location of the form:
> +
> +     arg_0 op arg_1
> +     ~~~~~ ^~ ~~~~~
> +       |        |
> +       |        arg1 type
> +       arg0 type
> +
> +   labelling the types of the arguments if SHOW_TYPES is true.
> +
> +   Otherwise, make a 1-location rich_location using the compound
> +   location within LOC:
> +
> +     arg_0 op arg_1
> +     ~~~~~~^~~~~~~~
> +
> +   for which we can't label the types.  */
> +
> +binary_op_rich_location::binary_op_rich_location (const
> op_location_t &loc,
> +						  tree arg0, tree
> arg1,
> +						  bool show_types)
> +: gcc_rich_location (loc.m_combined_loc),
> +  m_label_for_arg0 (arg0, arg1),
> +  m_label_for_arg1 (arg1, arg0)
> +{
> +  /* Default (above) to using the combined loc.
> +     Potentially override it here: if we have location information
> for the
> +     operator and for both arguments, then split them all out.
> +     Alternatively, override it if we don't have the combined
> location.  */
> +  if (use_operator_loc_p (loc, arg0, arg1))
> +    {
> +      set_range (0, loc.m_operator_loc, SHOW_RANGE_WITH_CARET);
> +      maybe_add_expr (arg0, show_types ? &m_label_for_arg0 : NULL);
> +      maybe_add_expr (arg1, show_types ? &m_label_for_arg1 : NULL);
> +    }
> +}
> +
> +/* Determine if binary_op_rich_location's ctor should attempt to
> make
> +   a 3-location rich_location (the location of the operator and of
> +   the 2 arguments), or fall back to a 1-location rich_location
> showing
> +   just the combined location of the operation as a whole.  */
> +
> +bool
> +binary_op_rich_location::use_operator_loc_p (const op_location_t
> &loc,
> +					     tree arg0, tree arg1)
> +{
> +  /* If we don't have a combined location, then use the operator
> location,
> +     and try to add ranges for the operators.  */
> +  if (loc.m_combined_loc == UNKNOWN_LOCATION)
> +    return true;
> +
> +  /* If we don't have the operator location, then use the
> +     combined location.  */
> +  if (loc.m_operator_loc == UNKNOWN_LOCATION)
> +    return false;
> +
> +  /* We have both operator location and combined location: only use
> the
> +     operator location if we have locations for both arguments.  */
> +  return (EXPR_HAS_LOCATION (arg0)
> +	  && EXPR_HAS_LOCATION (arg1));
> +}
> diff --git a/gcc/gcc-rich-location.h b/gcc/gcc-rich-location.h
> index 200bbb5..202d4f4 100644
> --- a/gcc/gcc-rich-location.h
> +++ b/gcc/gcc-rich-location.h
> @@ -162,4 +162,61 @@ class range_label_for_type_mismatch : public
> range_label
>    tree m_other_type;
>  };
>  
> +/* Subclass of range_label for labelling the type of EXPR when
> reporting
> +   a type mismatch between EXPR and OTHER_EXPR.
> +   Either or both of EXPR and OTHER_EXPR could be NULL.  */
> +
> +class maybe_range_label_for_tree_type_mismatch : public range_label
> +{
> + public:
> +  maybe_range_label_for_tree_type_mismatch (tree expr, tree
> other_expr)
> +  : m_expr (expr), m_other_expr (other_expr)
> +  {
> +  }
> +
> +  label_text get_text (unsigned range_idx) const FINAL OVERRIDE;
> +
> + private:
> +  tree m_expr;
> +  tree m_other_expr;
> +};
> +
> +struct op_location_t;
> +
> +/* A subclass of rich_location for showing problems with binary
> operations.
> +
> +   If enough location information is available, the ctor will make a
> +   3-location rich_location of the form:
> +
> +     arg_0 op arg_1
> +     ~~~~~ ^~ ~~~~~
> +       |        |
> +       |        arg1 type
> +       arg0 type
> +
> +   labelling the types of the arguments if SHOW_TYPES is true.
> +
> +   Otherwise, it will fall back to a 1-location rich_location using
> the
> +   compound location within LOC:
> +
> +     arg_0 op arg_1
> +     ~~~~~~^~~~~~~~
> +
> +   for which we can't label the types.  */
> +
> +class binary_op_rich_location : public gcc_rich_location
> +{
> + public:
> +  binary_op_rich_location (const op_location_t &loc,
> +			   tree arg0, tree arg1,
> +			   bool show_types);
> +
> + private:
> +  static bool use_operator_loc_p (const op_location_t &loc,
> +				  tree arg0, tree arg1);
> +
> +  maybe_range_label_for_tree_type_mismatch m_label_for_arg0;
> +  maybe_range_label_for_tree_type_mismatch m_label_for_arg1;
> +};
> +
>  #endif /* GCC_RICH_LOCATION_H */
> diff --git a/gcc/testsuite/c-c++-common/Wtautological-compare-
> ranges.c b/gcc/testsuite/c-c++-common/Wtautological-compare-ranges.c
> new file mode 100644
> index 0000000..2634d27
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/Wtautological-compare-ranges.c
> @@ -0,0 +1,42 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Wtautological-compare -fdiagnostics-show-caret" }
> */
> +
> +#define FOO foo
> +
> +void
> +fn1 (int foo)
> +{
> +  if (foo == foo); /* { dg-warning "self-comparison always evaluates
> to true" } */
> +  /* { dg-begin-multiline-output "" }
> +   if (foo == foo);
> +           ^~
> +     { dg-end-multiline-output "" { target c } } */
> +  /* { dg-begin-multiline-output "" }
> +   if (foo == foo);
> +       ~~~ ^~ ~~~
> +     { dg-end-multiline-output "" { target c++ } } */
> +}
> +
> +void
> +fn2 (int foo)
> +{
> +  if (FOO == FOO); /* { dg-warning "self-comparison always evaluates
> to true" } */
> +  /* { dg-begin-multiline-output "" }
> +   if (FOO == FOO);
> +           ^~
> +     { dg-end-multiline-output "" } */
> +}
> +
> +void
> +fn3 (int foo)
> +{
> +  if ((foo & 16) == 10); /* { dg-warning "bitwise comparison always
> evaluates to false" } */
> +  /* { dg-begin-multiline-output "" }
> +   if ((foo & 16) == 10);
> +                  ^~
> +     { dg-end-multiline-output "" { target c } } */
> +  /* { dg-begin-multiline-output "" }
> +   if ((foo & 16) == 10);
> +       ~~~~~~~~~~ ^~ ~~
> +     { dg-end-multiline-output "" { target c++ } } */
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp0x/pr51420.C
> b/gcc/testsuite/g++.dg/cpp0x/pr51420.C
> index fc70d46..1612cef 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/pr51420.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/pr51420.C
> @@ -1,8 +1,18 @@
>  // { dg-do compile { target c++11 } }
> +// { dg-options "-fdiagnostics-show-caret" }
>  
>  void
>  foo()
>  {
>    float x = operator"" _F();  //  { dg-error  "13:'operator\"\"_F'
> was not declared in this scope" }
> +  /* { dg-begin-multiline-output "" }
> +   float x = operator"" _F();
> +             ^~~~~~~~~~~~~
> +     { dg-end-multiline-output "" } */
> +
>    float y = 0_F;  //  { dg-error  "unable to find numeric literal
> operator" }
> +  /* { dg-begin-multiline-output "" }
> +   float y = 0_F;
> +             ^~~
> +     { dg-end-multiline-output "" } */
>  }
> diff --git a/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C
> b/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C
> index 4ab7656..fab5849 100644
> --- a/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C
> +++ b/gcc/testsuite/g++.dg/diagnostic/bad-binary-ops.C
> @@ -11,7 +11,10 @@ void test_1 ()
>  
>  /* { dg-begin-multiline-output "" }
>     myvec[1] / ptr;
> -   ~~~~~~~~~^~~~~
> +   ~~~~~~~~ ^ ~~~
> +          |   |
> +          |   const int*
> +          __m128 {aka float}
>     { dg-end-multiline-output "" } */
>  }
>  
> @@ -28,8 +31,12 @@ int test_2 (void)
>  /* { dg-begin-multiline-output "" }
>     return (some_function ()
>             ~~~~~~~~~~~~~~~~
> +                         |
> +                         s
>      + some_other_function ());
> -    ^~~~~~~~~~~~~~~~~~~~~~~~
> +    ^ ~~~~~~~~~~~~~~~~~~~~~~
> +                          |
> +                          t
>     { dg-end-multiline-output "" } */
>  }
>  
> @@ -39,6 +46,52 @@ int test_3 (struct s param_s, struct t param_t)
>  
>  /* { dg-begin-multiline-output "" }
>     return param_s && param_t;
> +          ~~~~~~~ ^~ ~~~~~~~
> +          |          |
> +          s          t
> +   { dg-end-multiline-output "" } */
> +/* { dg-begin-multiline-output "" }
> +   return param_s && param_t;
>            ~~~~~~~~^~~~~~~~~~
>     { dg-end-multiline-output "" } */
>  }
> +
> +namespace ns_4
> +{
> +  struct s foo;
> +  namespace inner {
> +    struct t bar;
> +  };
> +};
> +
> +int test_4a (void)
> +{
> +  return ns_4::foo && ns_4::inner::bar; // { dg-error "no match for
> .operator" }
> +  /* { dg-begin-multiline-output "" }
> +   return ns_4::foo && ns_4::inner::bar;
> +          ~~~~~~~~~ ^~ ~~~~~~~~~~~~~~~~
> +                |                   |
> +                s                   t
> +     { dg-end-multiline-output "" } */
> +
> +  /* { dg-begin-multiline-output "" }
> +   return ns_4::foo && ns_4::inner::bar;
> +          ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> +     { dg-end-multiline-output "" } */
> +}
> +
> +int test_4b (void)
> +{
> +  return ::ns_4::foo && ns_4::inner::bar; // { dg-error "no match
> for .operator" }
> +  /* { dg-begin-multiline-output "" }
> +   return ::ns_4::foo && ns_4::inner::bar;
> +          ~~~~~~~~~~~ ^~ ~~~~~~~~~~~~~~~~
> +                  |                   |
> +                  s                   t
> +     { dg-end-multiline-output "" } */
> +
> +  /* { dg-begin-multiline-output "" }
> +   return ::ns_4::foo && ns_4::inner::bar;
> +          ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
> +     { dg-end-multiline-output "" } */
> +}
> diff --git a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
> b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
> index b19655d..de7570a 100644
> --- a/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
> +++ b/gcc/testsuite/g++.dg/diagnostic/param-type-mismatch-2.C
> @@ -204,7 +204,9 @@ int test_10 ()
>    return v10_a - v10_b; // { dg-error "no match for" }
>    /* { dg-begin-multiline-output "" }
>     return v10_a - v10_b;
> -          ~~~~~~^~~~~~~
> +          ~~~~~ ^ ~~~~~
> +          |       |
> +          s10     s10
>       { dg-end-multiline-output "" } */
>    // { dg-message "candidate" "" { target *-*-* } s10_operator }
>    /* { dg-begin-multiline-output "" }
> diff --git a/gcc/tree.h b/gcc/tree.h
> index bf685ed..5527ef1 100644
> --- a/gcc/tree.h
> +++ b/gcc/tree.h
> @@ -5959,4 +5959,53 @@ fndecl_built_in_p (const_tree node,
> built_in_function name)
>  	  && DECL_FUNCTION_CODE (node) == name);
>  }
>  
> +/* A struct for encapsulating location information about an operator
> +   and the operation built from it.
> +
> +   m_operator_loc is the location of the operator
> +   m_combined_loc is the location of the compound expression.
> +
> +   For example, given "a && b" the, operator location is:
> +      a && b
> +        ^~
> +   and the combined location is:
> +      a && b
> +      ~~^~~~
> +   Capturing this information allows for class
> binary_op_rich_location
> +   to provide detailed information about e.g. type mismatches in
> binary
> +   operations where enough location information is available:
> +
> +     arg_0 op arg_1
> +     ~~~~~ ^~ ~~~~~
> +       |        |
> +       |        arg1 type
> +       arg0 type
> +
> +   falling back to just showing the combined location:
> +
> +     arg_0 op arg_1
> +     ~~~~~~^~~~~~~~
> +
> +   where it is not.  */
> +
> +struct op_location_t
> +{
> +  location_t m_operator_loc;
> +  location_t m_combined_loc;
> +
> +  /* 1-argument ctor, for constructing from a combined location.  */
> +  op_location_t (location_t combined_loc)
> +  : m_operator_loc (UNKNOWN_LOCATION), m_combined_loc (combined_loc)
> +  {}
> +
> +  /* 2-argument ctor, for distinguishing between the operator's
> location
> +     and the combined location.  */
> +  op_location_t (location_t operator_loc, location_t combined_loc)
> +  : m_operator_loc (operator_loc), m_combined_loc (combined_loc)
> +  {}
> +
> +  /* Implicitly convert back to a location_t, using the combined
> location.  */
> +  operator location_t () const { return m_combined_loc; }
> +};
> +
>  #endif  /* GCC_TREE_H  */


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