public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 2/2] C: use full locations within c_parser_expr_list's vec<location_t>
  2017-08-19 20:23 [PATCH 1/2] c-family/c/c++: pass optional vec<location_t> to c-format.c David Malcolm
@ 2017-08-19 18:22 ` David Malcolm
  2017-08-21 15:32   ` Joseph Myers
  2017-08-22 11:40   ` Andreas Schwab
  0 siblings, 2 replies; 7+ messages in thread
From: David Malcolm @ 2017-08-19 18:22 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

The previous patch uncovered a bug in how c_parser_expr_list builds the
vec<location_t>: it was only using the location of the first token
within each assignment-expression in the expr-list.

This shows up in e.g. this -Wformat warning, where only part of the
2nd param is underlined:

   printf("hello %i", (long)0);
                 ~^   ~
                 %li

This patch fixes c_parser_expr_list to use the full range of
each assignment-expression in the list for the vec<location_t>, so
that for the above we print:

   printf("hello %i", (long)0);
                 ~^   ~~~~~~~
                 %li

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/c/ChangeLog:
	* c-parser.c (c_parser_expr_list): Use c_expr::get_location ()
	rather than peeking the location of the first token.
	* c-tree.h (c_expr::get_location): New method.

gcc/testsuite/ChangeLog:
	* gcc.dg/format/diagnostic-ranges.c (test_mismatching_types):
	Update expected result to show all of "(long)0" being underlined.
	* gcc.dg/plugin/diagnostic-test-string-literals-1.c
	(test_multitoken_macro): Update expected underlining.
---
 gcc/c/c-parser.c                                              | 11 +++++------
 gcc/c/c-tree.h                                                |  8 ++++++++
 gcc/testsuite/gcc.dg/format/diagnostic-ranges.c               |  2 +-
 .../gcc.dg/plugin/diagnostic-test-string-literals-1.c         |  2 +-
 4 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 1402ba6..dfc75e0 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -8933,7 +8933,6 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
   vec<tree, va_gc> *ret;
   vec<tree, va_gc> *orig_types;
   struct c_expr expr;
-  location_t loc = c_parser_peek_token (parser)->location;
   unsigned int idx = 0;
 
   ret = make_tree_vector ();
@@ -8946,14 +8945,14 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
     c_parser_check_literal_zero (parser, literal_zero_mask, 0);
   expr = c_parser_expr_no_commas (parser, NULL);
   if (convert_p)
-    expr = convert_lvalue_to_rvalue (loc, expr, true, true);
+    expr = convert_lvalue_to_rvalue (expr.get_location (), expr, true, true);
   if (fold_p)
     expr.value = c_fully_fold (expr.value, false, NULL);
   ret->quick_push (expr.value);
   if (orig_types)
     orig_types->quick_push (expr.original_type);
   if (locations)
-    locations->safe_push (loc);
+    locations->safe_push (expr.get_location ());
   if (sizeof_arg != NULL
       && expr.original_code == SIZEOF_EXPR)
     {
@@ -8963,19 +8962,19 @@ c_parser_expr_list (c_parser *parser, bool convert_p, bool fold_p,
   while (c_parser_next_token_is (parser, CPP_COMMA))
     {
       c_parser_consume_token (parser);
-      loc = c_parser_peek_token (parser)->location;
       if (literal_zero_mask)
 	c_parser_check_literal_zero (parser, literal_zero_mask, idx + 1);
       expr = c_parser_expr_no_commas (parser, NULL);
       if (convert_p)
-	expr = convert_lvalue_to_rvalue (loc, expr, true, true);
+	expr = convert_lvalue_to_rvalue (expr.get_location (), expr, true,
+					 true);
       if (fold_p)
 	expr.value = c_fully_fold (expr.value, false, NULL);
       vec_safe_push (ret, expr.value);
       if (orig_types)
 	vec_safe_push (orig_types, expr.original_type);
       if (locations)
-	locations->safe_push (loc);
+	locations->safe_push (expr.get_location ());
       if (++idx < 3
 	  && sizeof_arg != NULL
 	  && expr.original_code == SIZEOF_EXPR)
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 92bcc70..5182cc5 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -147,6 +147,14 @@ struct c_expr
   location_t get_start () const { return src_range.m_start; }
   location_t get_finish () const { return src_range.m_finish; }
 
+  location_t get_location () const
+  {
+    if (CAN_HAVE_LOCATION_P (value))
+      return EXPR_LOCATION (value);
+    else
+      return make_location (get_start (), get_start (), get_finish ());
+  }
+
   /* Set the value to error_mark_node whilst ensuring that src_range
      is initialized.  */
   void set_error ()
diff --git a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
index bc6f665..e56e159 100644
--- a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
+++ b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
@@ -25,7 +25,7 @@ void test_mismatching_types (const char *msg)
   printf("hello %i", (long)0);  /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'long int' " } */
 /* { dg-begin-multiline-output "" }
    printf("hello %i", (long)0);
-                 ~^   ~
+                 ~^   ~~~~~~~
                  %li
    { dg-end-multiline-output "" } */
 }
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c
index 03f042a..bea86af 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c
@@ -250,7 +250,7 @@ test_multitoken_macro (void)
   __emit_string_literal_range (RANGE, 4, 3, 6);
 /* { dg-begin-multiline-output "" }
  #define RANGE ("0123456789")
-               ^
+               ^~~~~~~~~~~~~~
    { dg-end-multiline-output "" } */
 /* { dg-begin-multiline-output "" }
    __emit_string_literal_range (RANGE, 4, 3, 6);
-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] c-family/c/c++: pass optional vec<location_t> to c-format.c
@ 2017-08-19 20:23 David Malcolm
  2017-08-19 18:22 ` [PATCH 2/2] C: use full locations within c_parser_expr_list's vec<location_t> David Malcolm
  0 siblings, 1 reply; 7+ messages in thread
From: David Malcolm @ 2017-08-19 20:23 UTC (permalink / raw)
  To: gcc-patches; +Cc: David Malcolm

This patch passes along the vec<location_t> of argument locations
at a callsite from the C frontend to check_function_arguments and
from there to c-format.c, so that we can underline the pertinent
argument to mismatched format codes even for tree codes like decls
and constants which lack a location_t for their usage sites.

This takes e.g.:

    printf("hello %i %i %i ", foo, bar, baz);
                     ~^
                     %s

to:

    printf("hello %i %i %i ", foo, bar, baz);
                     ~^            ~~~
                     %s

which is useful for cases where there's more than one variadic argument.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu.

OK for trunk?

gcc/c-family/ChangeLog:
	* c-common.c (check_function_arguments): Add "arglogs" param; pass
	it to check_function_format.
	* c-common.h (check_function_arguments): Add vec<location_t> *
	param.
	(check_function_format): Likewise.
	* c-format.c (struct format_check_context): Add field "arglocs".
	(check_function_format): Add param "arglocs"; pass it to
	check_format_info.
	(check_format_info):  Add param "arglocs"; use it to initialize
	new field of format_ctx.
	(check_format_arg): Pass format_ctx->arglocs to new param of
	check_format_info_main.
	(class argument_parser): New field "arglocs".
	(argument_parser::argument_parser): Add "arglocs_" param and use
	it to initialize new field.
	(argument_parser::check_argument_type): Pass new arglocs field to
	check_format_types.
	(check_format_info_main): Add param "arglocs", and use it when
	constructing arg_parser.
	(check_format_types): Add param "arglocs"; use it if non-NULL when
	!EXPR_HAS_LOCATION (cur_param) to get at location information.

gcc/c/ChangeLog:
	* c-typeck.c (build_function_call_vec): Pass arg_loc to call
	to check_function_arguments.

gcc/cp/ChangeLog:
	* call.c (build_over_call): Pass NULL for new parameter to
	check_function_arguments.
	* typeck.c (cp_build_function_call_vec): Likewise.

gcc/testsuite/ChangeLog:
	* gcc.dg/format/diagnostic-ranges.c: Update expected results
	to show underlining of all pertinent params.
	* gcc.dg/format/pr72858.c: Likewise.
---
 gcc/c-family/c-common.c                         |   4 +-
 gcc/c-family/c-common.h                         |   4 +-
 gcc/c-family/c-format.c                         |  52 ++++++++----
 gcc/c/c-typeck.c                                |   2 +-
 gcc/cp/call.c                                   |   2 +-
 gcc/cp/typeck.c                                 |   2 +-
 gcc/testsuite/gcc.dg/format/diagnostic-ranges.c |  41 ++++------
 gcc/testsuite/gcc.dg/format/pr72858.c           | 102 +++++++++---------------
 8 files changed, 98 insertions(+), 111 deletions(-)

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 4dc3b33..156c89d 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5539,7 +5539,7 @@ attribute_fallthrough_p (tree attr)
    diagnostics.  Return true if -Wnonnull warning has been diagnosed.  */
 bool
 check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
-			  int nargs, tree *argarray)
+			  int nargs, tree *argarray, vec<location_t> *arglocs)
 {
   bool warned_p = false;
 
@@ -5553,7 +5553,7 @@ check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
   /* Check for errors in format strings.  */
 
   if (warn_format || warn_suggest_attribute_format)
-    check_function_format (TYPE_ATTRIBUTES (fntype), nargs, argarray);
+    check_function_format (TYPE_ATTRIBUTES (fntype), nargs, argarray, arglocs);
 
   if (warn_format)
     check_function_sentinel (fntype, nargs, argarray);
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index 980d396..8e36768 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -808,7 +808,7 @@ extern tree fname_decl (location_t, unsigned, tree);
 
 extern int check_user_alignment (const_tree, bool);
 extern bool check_function_arguments (location_t loc, const_tree, const_tree,
-				      int, tree *);
+				      int, tree *, vec<location_t> *);
 extern void check_function_arguments_recurse (void (*)
 					      (void *, tree,
 					       unsigned HOST_WIDE_INT),
@@ -816,7 +816,7 @@ extern void check_function_arguments_recurse (void (*)
 					      unsigned HOST_WIDE_INT);
 extern bool check_builtin_function_arguments (location_t, vec<location_t>,
 					      tree, int, tree *);
-extern void check_function_format (tree, int, tree *);
+extern void check_function_format (tree, int, tree *, vec<location_t> *);
 extern bool attribute_fallthrough_p (tree);
 extern tree handle_format_attribute (tree *, tree, tree, int, bool *);
 extern tree handle_format_arg_attribute (tree *, tree, tree, int, bool *);
diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index 7c9095c..0dba979 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -989,6 +989,7 @@ struct format_check_context
   format_check_results *res;
   function_format_info *info;
   tree params;
+  vec<location_t> *arglocs;
 };
 
 /* Return the format name (as specified in the original table) for the format
@@ -1011,14 +1012,16 @@ format_flags (int format_num)
   gcc_unreachable ();
 }
 
-static void check_format_info (function_format_info *, tree);
+static void check_format_info (function_format_info *, tree,
+			       vec<location_t> *);
 static void check_format_arg (void *, tree, unsigned HOST_WIDE_INT);
 static void check_format_info_main (format_check_results *,
 				    function_format_info *, const char *,
 				    location_t, tree,
 				    int, tree,
 				    unsigned HOST_WIDE_INT,
-				    object_allocator<format_wanted_type> &);
+				    object_allocator<format_wanted_type> &,
+				    vec<location_t> *);
 
 static void init_dollar_format_checking (int, tree);
 static int maybe_read_dollar_number (const char **, int,
@@ -1033,7 +1036,8 @@ static void check_format_types (const substring_loc &fmt_loc,
 				format_wanted_type *,
 				const format_kind_info *fki,
 				int offset_to_type_start,
-				char conversion_char);
+				char conversion_char,
+				vec<location_t> *arglocs);
 static void format_type_warning (const substring_loc &fmt_loc,
 				 source_range *param_range,
 				 format_wanted_type *, tree,
@@ -1076,7 +1080,8 @@ decode_format_type (const char *s)
    attribute themselves.  */
 
 void
-check_function_format (tree attrs, int nargs, tree *argarray)
+check_function_format (tree attrs, int nargs, tree *argarray,
+		       vec<location_t> *arglocs)
 {
   tree a;
 
@@ -1097,7 +1102,7 @@ check_function_format (tree attrs, int nargs, tree *argarray)
 	      int i;
 	      for (i = nargs - 1; i >= 0; i--)
 		params = tree_cons (NULL_TREE, argarray[i], params);
-	      check_format_info (&info, params);
+	      check_format_info (&info, params, arglocs);
 	    }
 
 	  /* Attempt to detect whether the current function might benefit
@@ -1400,7 +1405,8 @@ get_flag_spec (const format_flag_spec *spec, int flag, const char *predicates)
    PARAMS is the list of argument values.  */
 
 static void
-check_format_info (function_format_info *info, tree params)
+check_format_info (function_format_info *info, tree params,
+		   vec<location_t> *arglocs)
 {
   format_check_context format_ctx;
   unsigned HOST_WIDE_INT arg_num;
@@ -1434,6 +1440,7 @@ check_format_info (function_format_info *info, tree params)
   format_ctx.res = &res;
   format_ctx.info = info;
   format_ctx.params = params;
+  format_ctx.arglocs = arglocs;
 
   check_function_arguments_recurse (check_format_arg, &format_ctx,
 				    format_tree, arg_num);
@@ -1518,6 +1525,7 @@ check_format_arg (void *ctx, tree format_tree,
   format_check_results *res = format_ctx->res;
   function_format_info *info = format_ctx->info;
   tree params = format_ctx->params;
+  vec<location_t> *arglocs = format_ctx->arglocs;
 
   int format_length;
   HOST_WIDE_INT offset;
@@ -1703,7 +1711,7 @@ check_format_arg (void *ctx, tree format_tree,
   res->number_other++;
   object_allocator <format_wanted_type> fwt_pool ("format_wanted_type pool");
   check_format_info_main (res, info, format_chars, fmt_param_loc, format_tree,
-			  format_length, params, arg_num, fwt_pool);
+			  format_length, params, arg_num, fwt_pool, arglocs);
 }
 
 /* Support class for argument_parser and check_format_info_main.
@@ -1768,7 +1776,8 @@ class argument_parser
 		   const char * const orig_format_chars,
 		   location_t format_string_loc, flag_chars_t &flag_chars,
 		   int &has_operand_number, tree first_fillin_param,
-		   object_allocator <format_wanted_type> &fwt_pool_);
+		   object_allocator <format_wanted_type> &fwt_pool_,
+		   vec<location_t> *arglocs);
 
   bool read_any_dollar ();
 
@@ -1847,6 +1856,7 @@ class argument_parser
  private:
   format_wanted_type *first_wanted_type;
   format_wanted_type *last_wanted_type;
+  vec<location_t> *arglocs;
 };
 
 /* flag_chars_t's constructor.  */
@@ -1997,7 +2007,8 @@ argument_parser (function_format_info *info_, const char *&format_chars_,
 		 flag_chars_t &flag_chars_,
 		 int &has_operand_number_,
 		 tree first_fillin_param_,
-		 object_allocator <format_wanted_type> &fwt_pool_)
+		 object_allocator <format_wanted_type> &fwt_pool_,
+		 vec<location_t> *arglocs_)
 : info (info_),
   fki (&format_types[info->format_type]),
   flag_specs (fki->flag_specs),
@@ -2013,7 +2024,8 @@ argument_parser (function_format_info *info_, const char *&format_chars_,
   has_operand_number (has_operand_number_),
   first_fillin_param (first_fillin_param_),
   first_wanted_type (NULL),
-  last_wanted_type (NULL)
+  last_wanted_type (NULL),
+  arglocs (arglocs_)
 {
 }
 
@@ -2736,7 +2748,7 @@ check_argument_type (const format_char_info *fci,
       ptrdiff_t offset_to_type_start = type_start - orig_format_chars;
       check_format_types (fmt_loc, first_wanted_type, fki,
 			  offset_to_type_start,
-			  conversion_char);
+			  conversion_char, arglocs);
     }
 
   return true;
@@ -2755,7 +2767,8 @@ check_format_info_main (format_check_results *res,
 			location_t fmt_param_loc, tree format_string_cst,
 			int format_length, tree params,
 			unsigned HOST_WIDE_INT arg_num,
-			object_allocator <format_wanted_type> &fwt_pool)
+			object_allocator <format_wanted_type> &fwt_pool,
+			vec<location_t> *arglocs)
 {
   const char * const orig_format_chars = format_chars;
   const tree first_fillin_param = params;
@@ -2802,7 +2815,7 @@ check_format_info_main (format_check_results *res,
       argument_parser arg_parser (info, format_chars, format_string_cst,
 				  orig_format_chars, format_string_loc,
 				  flag_chars, has_operand_number,
-				  first_fillin_param, fwt_pool);
+				  first_fillin_param, fwt_pool, arglocs);
 
       if (!arg_parser.read_any_dollar ())
 	return;
@@ -3032,7 +3045,8 @@ static void
 check_format_types (const substring_loc &fmt_loc,
 		    format_wanted_type *types, const format_kind_info *fki,
 		    int offset_to_type_start,
-		    char conversion_char)
+		    char conversion_char,
+		    vec<location_t> *arglocs)
 {
   for (; types != 0; types = types->next)
     {
@@ -3072,11 +3086,19 @@ check_format_types (const substring_loc &fmt_loc,
 
       source_range param_range;
       source_range *param_range_ptr;
-      if (CAN_HAVE_LOCATION_P (cur_param))
+      if (EXPR_HAS_LOCATION (cur_param))
 	{
 	  param_range = EXPR_LOCATION_RANGE (cur_param);
 	  param_range_ptr = &param_range;
 	}
+      else if (arglocs)
+	{
+	  /* arg_num is 1-based.  */
+	  gcc_assert (types->arg_num > 0);
+	  location_t param_loc = (*arglocs)[types->arg_num - 1];
+	  param_range = get_range_from_loc (line_table, param_loc);
+	  param_range_ptr = &param_range;
+	}
       else
 	param_range_ptr = NULL;
 
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index c33601f..d7ca148 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -3118,7 +3118,7 @@ build_function_call_vec (location_t loc, vec<location_t> arg_loc,
 
   /* Check that the arguments to the function are valid.  */
   bool warned_p = check_function_arguments (loc, fundecl, fntype,
-					    nargs, argarray);
+					    nargs, argarray, &arg_loc);
 
   if (name != NULL_TREE
       && !strncmp (IDENTIFIER_POINTER (name), "__builtin_", 10))
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 3790299..067db59a 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7949,7 +7949,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
 	fargs[j] = maybe_constant_value (argarray[j]);
 
       warned_p = check_function_arguments (input_location, fn, TREE_TYPE (fn),
-					   nargs, fargs);
+					   nargs, fargs, NULL);
     }
 
   if (DECL_INHERITED_CTOR (fn))
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index a5a363b..63667f3 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -3699,7 +3699,7 @@ cp_build_function_call_vec (tree function, vec<tree, va_gc> **params,
   /* Check for errors in format strings and inappropriately
      null parameters.  */
   bool warned_p = check_function_arguments (input_location, fndecl, fntype,
-					    nargs, argarray);
+					    nargs, argarray, NULL);
 
   ret = build_cxx_call (function, nargs, argarray, complain);
 
diff --git a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
index 8cce5b3..bc6f665 100644
--- a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
+++ b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
@@ -8,28 +8,24 @@ void test_mismatching_types (const char *msg)
 {
   printf("hello %i", msg);  /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
 
-/* TODO: ideally would also underline "msg".  */
 /* { dg-begin-multiline-output "" }
    printf("hello %i", msg);
-                 ~^
+                 ~^   ~~~
                  %s
    { dg-end-multiline-output "" } */
 
 
   printf("hello %s", 42);  /* { dg-warning "format '%s' expects argument of type 'char \\*', but argument 2 has type 'int'" } */
-/* TODO: ideally would also underline "42".  */
 /* { dg-begin-multiline-output "" }
    printf("hello %s", 42);
-                 ~^
+                 ~^   ~~
                  %d
    { dg-end-multiline-output "" } */
 
-
   printf("hello %i", (long)0);  /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'long int' " } */
-/* TODO: ideally would also underline the argument.  */
 /* { dg-begin-multiline-output "" }
    printf("hello %i", (long)0);
-                 ~^
+                 ~^   ~
                  %li
    { dg-end-multiline-output "" } */
 }
@@ -38,11 +34,12 @@ void test_multiple_arguments (void)
 {
   printf ("arg0: %i  arg1: %s arg 2: %i", /* { dg-warning "29: format '%s'" } */
           100, 101, 102);
-/* TODO: ideally would also underline "101".  */
 /* { dg-begin-multiline-output "" }
    printf ("arg0: %i  arg1: %s arg 2: %i",
                             ~^
                             %d
+           100, 101, 102);
+                ~~~           
    { dg-end-multiline-output "" } */
 }
 
@@ -84,10 +81,9 @@ void test_hex (const char *msg)
      "i" is \x69 */
   printf("hello \x25\x69", msg);  /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
 
-/* TODO: ideally would also underline "msg".  */
 /* { dg-begin-multiline-output "" }
    printf("hello \x25\x69", msg);
-                 ~~~~^~~~
+                 ~~~~^~~~   ~~~
                  \x25s
    { dg-end-multiline-output "" } */
 }
@@ -98,10 +94,9 @@ void test_oct (const char *msg)
      "i" is octal 151.  */
   printf("hello \045\151", msg);  /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
 
-/* TODO: ideally would also underline "msg".  */
 /* { dg-begin-multiline-output "" }
    printf("hello \045\151", msg);
-                 ~~~~^~~~
+                 ~~~~^~~~   ~~~
                  \045s
    { dg-end-multiline-output "" } */
 }
@@ -115,9 +110,10 @@ void test_multiple (const char *msg)
 /* { dg-begin-multiline-output "" }
    printf("prefix"  "\x25"  "\151"  "suffix",
           ^~~~~~~~
+          msg);
+          ~~~
   { dg-end-multiline-output "" } */
 
-/* TODO: ideally would also underline "msg".  */
 /* { dg-begin-multiline-output "" }
    printf("prefix"  "\x25"  "\151"  "suffix",
                      ~~~~~~~~^~~~
@@ -128,10 +124,9 @@ void test_multiple (const char *msg)
 void test_u8 (const char *msg)
 {
   printf(u8"hello %i", msg);/* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
-/* TODO: ideally would also underline "msg".  */
 /* { dg-begin-multiline-output "" }
    printf(u8"hello %i", msg);
-                   ~^
+                   ~^   ~~~
                    %s
    { dg-end-multiline-output "" } */
 }
@@ -151,7 +146,7 @@ void test_field_width_specifier (long l, int i1, int i2)
   printf (" %*.*d ", l, i1, i2); /* { dg-warning "14: field width specifier '\\*' expects argument of type 'int', but argument 2 has type 'long int'" } */
 /* { dg-begin-multiline-output "" }
    printf (" %*.*d ", l, i1, i2);
-             ~^~~~
+             ~^~~~    ~
    { dg-end-multiline-output "" } */
 }
 
@@ -160,10 +155,9 @@ void test_field_width_specifier (long l, int i1, int i2)
 void test_field_width_specifier_2 (char *d, long foo, long bar)
 {
   __builtin_sprintf (d, " %*ld ", foo, foo); /* { dg-warning "28: field width specifier '\\*' expects argument of type 'int', but argument 3 has type 'long int'" } */
-  /* TODO: ideally we'd underline the first "foo" here".  */
   /* { dg-begin-multiline-output "" }
    __builtin_sprintf (d, " %*ld ", foo, foo);
-                           ~^~~
+                           ~^~~    ~~~
    { dg-end-multiline-output "" } */
 
   __builtin_sprintf (d, " %*ld ", foo + bar, foo); /* { dg-warning "28: field width specifier '\\*' expects argument of type 'int', but argument 3 has type 'long int'" } */
@@ -176,10 +170,9 @@ void test_field_width_specifier_2 (char *d, long foo, long bar)
 void test_field_precision_specifier (char *d, long foo, long bar)
 {
   __builtin_sprintf (d, " %.*ld ", foo, foo); /* { dg-warning "29: field precision specifier '\\.\\*' expects argument of type 'int', but argument 3 has type 'long int'" } */
-  /* TODO: ideally we'd underline the first "foo" here".  */
   /* { dg-begin-multiline-output "" }
    __builtin_sprintf (d, " %.*ld ", foo, foo);
-                           ~~^~~
+                           ~~^~~    ~~~
    { dg-end-multiline-output "" } */
 
   __builtin_sprintf (d, " %.*ld ", foo + bar, foo); /* { dg-warning "29: field precision specifier '\\.\\*' expects argument of type 'int', but argument 3 has type 'long int'" } */
@@ -247,7 +240,7 @@ void test_macro (const char *msg)
   printf("hello " INT_FMT " world", msg);  /* { dg-warning "10: format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
 /* { dg-begin-multiline-output "" }
    printf("hello " INT_FMT " world", msg);
-          ^~~~~~~~
+          ^~~~~~~~                   ~~~
    { dg-end-multiline-output "" } */
 /* { dg-begin-multiline-output "" }
  #define INT_FMT "%i"
@@ -263,7 +256,7 @@ void test_macro_2 (const char *msg)
   printf("hello %" PRIu32 " world", msg);  /* { dg-warning "10: format '%u' expects argument of type 'unsigned int', but argument 2 has type 'const char \\*' " } */
 /* { dg-begin-multiline-output "" }
    printf("hello %" PRIu32 " world", msg);
-          ^~~~~~~~~
+          ^~~~~~~~~                  ~~~
    { dg-end-multiline-output "" } */
 /* { dg-begin-multiline-output "" }
  #define PRIu32 "u"
@@ -313,7 +306,7 @@ void test_non_contiguous_strings (void)
                                     /* { dg-message "26: format string is defined here" "" { target *-*-* } .-1 } */
   /* { dg-begin-multiline-output "" }
    __builtin_printf(" %" "d ", 0.5);
-                    ^~~~
+                    ^~~~       ~~~
    { dg-end-multiline-output "" } */
   /* { dg-begin-multiline-output "" }
    __builtin_printf(" %" "d ", 0.5);
@@ -330,6 +323,6 @@ void test_const_arrays (void)
   __builtin_printf(a, 0.5); /* { dg-warning "20: format .%d. expects argument of type .int., but argument 2 has type .double." } */
   /* { dg-begin-multiline-output "" }
    __builtin_printf(a, 0.5);
-                    ^
+                    ^  ~~~
    { dg-end-multiline-output "" } */
 }
diff --git a/gcc/testsuite/gcc.dg/format/pr72858.c b/gcc/testsuite/gcc.dg/format/pr72858.c
index c142d24..b8c5829 100644
--- a/gcc/testsuite/gcc.dg/format/pr72858.c
+++ b/gcc/testsuite/gcc.dg/format/pr72858.c
@@ -25,56 +25,49 @@ test_x (char *d,
   sprintf (d, " %-8x ", uiexpr);
 
   sprintf (d, " %-8x ", lexpr); /* { dg-warning "20: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'long int'" } */
-/* TODO: ideally would also underline "lexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8x ", lexpr);
-                 ~~~^
+                 ~~~^    ~~~~~
                  %-8lx
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8x ", ulexpr); /* { dg-warning "20: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int'" } */
-/* TODO: ideally would also underline "lexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8x ", ulexpr);
-                 ~~~^
+                 ~~~^    ~~~~~~
                  %-8lx
    { dg-end-multiline-output "" } */
 
   sprintf (d, " %-8x ", llexpr); /* { dg-warning "20: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'long long int'" } */
-/* TODO: ideally would also underline "lexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8x ", llexpr);
-                 ~~~^
+                 ~~~^    ~~~~~~
                  %-8llx
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8x ", ullexpr); /* { dg-warning "20: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'long long unsigned int'" } */
-/* TODO: ideally would also underline "lexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8x ", ullexpr);
-                 ~~~^
+                 ~~~^    ~~~~~~~
                  %-8llx
    { dg-end-multiline-output "" } */
 
   /* Floating-point arguments.  */
 
   sprintf (d, " %-8x ", fexpr); /* { dg-warning "20: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'double'" } */
-/* TODO: ideally would also underline "fexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8x ", fexpr);
-                 ~~~^
+                 ~~~^    ~~~~~
                  %-8f
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8x ", dexpr); /* { dg-warning "20: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'double'" } */
-/* TODO: ideally would also underline "dexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8x ", dexpr);
-                 ~~~^
+                 ~~~^    ~~~~~
                  %-8f
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8x ", ldexpr); /* { dg-warning "20: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'long double'" } */
-/* TODO: ideally would also underline "ldexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8x ", ldexpr);
-                 ~~~^
+                 ~~~^    ~~~~~~
                  %-8Lf
    { dg-end-multiline-output "" } */
 
@@ -82,7 +75,7 @@ test_x (char *d,
   sprintf (d, " %-8x ", ptr); /* { dg-warning "20: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'void \\*'" } */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8x ", ptr);
-                 ~~~^
+                 ~~~^    ~~~
                  %-8p
    { dg-end-multiline-output "" } */
 
@@ -92,7 +85,7 @@ test_x (char *d,
   sprintf (d, " %-8x ", s); /* { dg-warning "20: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'struct s'" } */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8x ", s);
-                 ~~~^
+                 ~~~^    ~
    { dg-end-multiline-output "" } */
 }
 
@@ -109,17 +102,15 @@ test_lx (char *d,
   /* Integer arguments.  */
 
   sprintf (d, " %-8lx ", iexpr); /* { dg-warning "21: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'int'" } */
-/* TODO: ideally would also underline "iexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lx ", iexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8x
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8lx ", uiexpr); /* { dg-warning "21: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int'" } */
-/* TODO: ideally would also underline "uiexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lx ", uiexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~~
                  %-8x
    { dg-end-multiline-output "" } */
 
@@ -127,41 +118,36 @@ test_lx (char *d,
   sprintf (d, " %-8lx ", ulexpr);
 
   sprintf (d, " %-8lx ", llexpr); /* { dg-warning "21: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'long long int'" } */
-/* TODO: ideally would also underline "llexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lx ", llexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~~
                  %-8llx
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8lx ", ullexpr); /* { dg-warning "21: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'long long unsigned int'" } */
-/* TODO: ideally would also underline "ullexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lx ", ullexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~~~
                  %-8llx
    { dg-end-multiline-output "" } */
 
   /* Floating-point arguments.  */
 
   sprintf (d, " %-8lx ", fexpr); /* { dg-warning "21: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'double'" } */
-/* TODO: ideally would also underline "fexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lx ", fexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8f
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8lx ", dexpr); /* { dg-warning "21: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'double'" } */
-/* TODO: ideally would also underline "dexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lx ", dexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8f
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8lx ", ldexpr); /* { dg-warning "21: format '%lx' expects argument of type 'long unsigned int', but argument 3 has type 'long double'" } */
-/* TODO: ideally would also underline "ldexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lx ", ldexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~~
                  %-8Lf
    { dg-end-multiline-output "" } */
 }
@@ -181,32 +167,28 @@ test_o (char *d,
   sprintf (d, " %-8o ", uiexpr);
 
   sprintf (d, " %-8o ", lexpr); /* { dg-warning "20: format '%o' expects argument of type 'unsigned int', but argument 3 has type 'long int'" } */
-/* TODO: ideally would also underline "lexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8o ", lexpr);
-                 ~~~^
+                 ~~~^    ~~~~~
                  %-8lo
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8o ", ulexpr); /* { dg-warning "20: format '%o' expects argument of type 'unsigned int', but argument 3 has type 'long unsigned int'" } */
-/* TODO: ideally would also underline "lexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8o ", ulexpr);
-                 ~~~^
+                 ~~~^    ~~~~~~
                  %-8lo
    { dg-end-multiline-output "" } */
 
   sprintf (d, " %-8o ", llexpr); /* { dg-warning "20: format '%o' expects argument of type 'unsigned int', but argument 3 has type 'long long int'" } */
-/* TODO: ideally would also underline "lexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8o ", llexpr);
-                 ~~~^
+                 ~~~^    ~~~~~~
                  %-8llo
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8o ", ullexpr); /* { dg-warning "20: format '%o' expects argument of type 'unsigned int', but argument 3 has type 'long long unsigned int'" } */
-/* TODO: ideally would also underline "lexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8o ", ullexpr);
-                 ~~~^
+                 ~~~^    ~~~~~~~
                  %-8llo
    { dg-end-multiline-output "" } */
 }
@@ -223,17 +205,15 @@ test_lo (char *d,
   /* Integer arguments.  */
 
   sprintf (d, " %-8lo ", iexpr); /* { dg-warning "21: format '%lo' expects argument of type 'long unsigned int', but argument 3 has type 'int'" } */
-/* TODO: ideally would also underline "iexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lo ", iexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8o
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8lo ", uiexpr); /* { dg-warning "21: format '%lo' expects argument of type 'long unsigned int', but argument 3 has type 'unsigned int'" } */
-/* TODO: ideally would also underline "uiexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lo ", uiexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~~
                  %-8o
    { dg-end-multiline-output "" } */
 
@@ -241,17 +221,15 @@ test_lo (char *d,
   sprintf (d, " %-8lo ", ulexpr);
 
   sprintf (d, " %-8lo ", llexpr); /* { dg-warning "21: format '%lo' expects argument of type 'long unsigned int', but argument 3 has type 'long long int'" } */
-/* TODO: ideally would also underline "llexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lo ", llexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~~
                  %-8llo
    { dg-end-multiline-output "" } */
   sprintf (d, " %-8lo ", ullexpr); /* { dg-warning "21: format '%lo' expects argument of type 'long unsigned int', but argument 3 has type 'long long unsigned int'" } */
-/* TODO: ideally would also underline "ullexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8lo ", ullexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~~~
                  %-8llo
    { dg-end-multiline-output "" } */
 }
@@ -265,10 +243,9 @@ test_e (char *d, int iexpr, float fexpr, double dexpr, long double ldexpr)
   /* Integer arguments.  */
 
   sprintf (d, " %-8e ", iexpr); /* { dg-warning "20: format '%e' expects argument of type 'double', but argument 3 has type 'int'" } */
-/* TODO: ideally would also underline "iexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8e ", iexpr);
-                 ~~~^
+                 ~~~^    ~~~~~
                  %-8d
    { dg-end-multiline-output "" } */
 
@@ -277,10 +254,9 @@ test_e (char *d, int iexpr, float fexpr, double dexpr, long double ldexpr)
   sprintf (d, " %-8e ", fexpr);
   sprintf (d, " %-8e ", dexpr);
   sprintf (d, " %-8e ", ldexpr); /* { dg-warning "20: format '%e' expects argument of type 'double', but argument 3 has type 'long double'" } */
-/* TODO: ideally would also underline "ldexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8e ", ldexpr);
-                 ~~~^
+                 ~~~^    ~~~~~~
                  %-8Le
    { dg-end-multiline-output "" } */
 }
@@ -294,10 +270,9 @@ test_Le (char *d, int iexpr, float fexpr, double dexpr, long double ldexpr)
   /* Integer arguments.  */
 
   sprintf (d, " %-8Le ", iexpr); /* { dg-warning "21: format '%Le' expects argument of type 'long double', but argument 3 has type 'int'" } */
-/* TODO: ideally would also underline "iexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8Le ", iexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8d
    { dg-end-multiline-output "" } */
 
@@ -306,14 +281,14 @@ test_Le (char *d, int iexpr, float fexpr, double dexpr, long double ldexpr)
   sprintf (d, " %-8Le ", fexpr); /* { dg-warning "21: format '%Le' expects argument of type 'long double', but argument 3 has type 'double'" } */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8Le ", fexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8e
    { dg-end-multiline-output "" } */
 
   sprintf (d, " %-8Le ", dexpr); /* { dg-warning "21: format '%Le' expects argument of type 'long double', but argument 3 has type 'double'" } */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8Le ", dexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8e
    { dg-end-multiline-output "" } */
 
@@ -329,10 +304,9 @@ test_E (char *d, int iexpr, float fexpr, double dexpr, long double ldexpr)
   /* Integer arguments.  */
 
   sprintf (d, " %-8E ", iexpr); /* { dg-warning "20: format '%E' expects argument of type 'double', but argument 3 has type 'int'" } */
-/* TODO: ideally would also underline "iexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8E ", iexpr);
-                 ~~~^
+                 ~~~^    ~~~~~
                  %-8d
    { dg-end-multiline-output "" } */
 
@@ -341,10 +315,9 @@ test_E (char *d, int iexpr, float fexpr, double dexpr, long double ldexpr)
   sprintf (d, " %-8E ", fexpr);
   sprintf (d, " %-8E ", dexpr);
   sprintf (d, " %-8E ", ldexpr); /* { dg-warning "20: format '%E' expects argument of type 'double', but argument 3 has type 'long double'" } */
-/* TODO: ideally would also underline "ldexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8E ", ldexpr);
-                 ~~~^
+                 ~~~^    ~~~~~~
                  %-8LE
    { dg-end-multiline-output "" } */
 }
@@ -358,24 +331,23 @@ test_LE (char *d, int iexpr, float fexpr, double dexpr, long double ldexpr)
   /* Integer arguments.  */
 
   sprintf (d, " %-8LE ", iexpr); /* { dg-warning "21: format '%LE' expects argument of type 'long double', but argument 3 has type 'int'" } */
-/* TODO: ideally would also underline "iexpr".  */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8LE ", iexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8d
    { dg-end-multiline-output "" } */
 
   sprintf (d, " %-8LE ", fexpr); /* { dg-warning "21: format '%LE' expects argument of type 'long double', but argument 3 has type 'double'" } */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8LE ", fexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8E
    { dg-end-multiline-output "" } */
 
   sprintf (d, " %-8LE ", dexpr); /* { dg-warning "21: format '%LE' expects argument of type 'long double', but argument 3 has type 'double'" } */
 /* { dg-begin-multiline-output "" }
    sprintf (d, " %-8LE ", dexpr);
-                 ~~~~^
+                 ~~~~^    ~~~~~
                  %-8E
    { dg-end-multiline-output "" } */
 
@@ -394,19 +366,19 @@ test_everything (char *d, long lexpr)
   /* { dg-warning "26: field width specifier '\\*' expects argument of type 'int', but argument 3 has type 'long int'" "" { target *-*-* } test_everything_sprintf } */
   /* { dg-begin-multiline-output "" }
    sprintf (d, "before %-+*.*lld after", lexpr, lexpr, lexpr);
-                       ~~~^~~~~~
+                       ~~~^~~~~~         ~~~~~
    { dg-end-multiline-output "" } */
 
   /* { dg-warning "28: field precision specifier '\\.\\*' expects argument of type 'int', but argument 4 has type 'long int'" "" { target *-*-* } test_everything_sprintf } */
   /* { dg-begin-multiline-output "" }
    sprintf (d, "before %-+*.*lld after", lexpr, lexpr, lexpr);
-                       ~~~~~^~~~
+                       ~~~~~^~~~                ~~~~~
    { dg-end-multiline-output "" } */
 
   /* { dg-warning "31: format '%lld' expects argument of type 'long long int', but argument 5 has type 'long int'" "" { target *-*-* } test_everything_sprintf } */
   /* { dg-begin-multiline-output "" }
    sprintf (d, "before %-+*.*lld after", lexpr, lexpr, lexpr);
-                       ~~~~~~~~^
+                       ~~~~~~~~^                       ~~~~~
                        %-+*.*ld
    { dg-end-multiline-output "" } */
 }
-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] C: use full locations within c_parser_expr_list's vec<location_t>
  2017-08-19 18:22 ` [PATCH 2/2] C: use full locations within c_parser_expr_list's vec<location_t> David Malcolm
@ 2017-08-21 15:32   ` Joseph Myers
  2017-08-22 11:40   ` Andreas Schwab
  1 sibling, 0 replies; 7+ messages in thread
From: Joseph Myers @ 2017-08-21 15:32 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc-patches

These two patches are OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] C: use full locations within c_parser_expr_list's vec<location_t>
  2017-08-19 18:22 ` [PATCH 2/2] C: use full locations within c_parser_expr_list's vec<location_t> David Malcolm
  2017-08-21 15:32   ` Joseph Myers
@ 2017-08-22 11:40   ` Andreas Schwab
  2017-08-23 16:01     ` David Malcolm
  2017-08-23 19:35     ` [PATCH] C: fix logic within c_expr::get_location David Malcolm
  1 sibling, 2 replies; 7+ messages in thread
From: Andreas Schwab @ 2017-08-22 11:40 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc-patches

On Aug 18 2017, David Malcolm <dmalcolm@redhat.com> wrote:

> gcc/c/ChangeLog:
> 	* c-parser.c (c_parser_expr_list): Use c_expr::get_location ()
> 	rather than peeking the location of the first token.
> 	* c-tree.h (c_expr::get_location): New method.

This breaks gcc.dg/Wtraditional-conversion-2.c on m68k:

FAIL: gcc.dg/Wtraditional-conversion-2.c  (test for warnings, line 54)
FAIL: gcc.dg/Wtraditional-conversion-2.c  (test for warnings, line 55)
FAIL: gcc.dg/Wtraditional-conversion-2.c (test for excess errors)
Excess errors:
cc1: warning: passing argument 1 of 'ff' as 'float' rather than 'double' due to prototype
cc1: warning: passing argument 1 of 'x.ff' as 'float' rather than 'double' due to prototype

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] C: use full locations within c_parser_expr_list's vec<location_t>
  2017-08-22 11:40   ` Andreas Schwab
@ 2017-08-23 16:01     ` David Malcolm
  2017-08-23 19:35     ` [PATCH] C: fix logic within c_expr::get_location David Malcolm
  1 sibling, 0 replies; 7+ messages in thread
From: David Malcolm @ 2017-08-23 16:01 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: gcc-patches

On Tue, 2017-08-22 at 13:01 +0200, Andreas Schwab wrote:
> On Aug 18 2017, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> > gcc/c/ChangeLog:
> > 	* c-parser.c (c_parser_expr_list): Use c_expr::get_location ()
> > 	rather than peeking the location of the first token.
> > 	* c-tree.h (c_expr::get_location): New method.
> 
> This breaks gcc.dg/Wtraditional-conversion-2.c on m68k:
> 
> FAIL: gcc.dg/Wtraditional-conversion-2.c  (test for warnings, line
> 54)
> FAIL: gcc.dg/Wtraditional-conversion-2.c  (test for warnings, line
> 55)
> FAIL: gcc.dg/Wtraditional-conversion-2.c (test for excess errors)
> Excess errors:
> cc1: warning: passing argument 1 of 'ff' as 'float' rather than
> 'double' due to prototype
> cc1: warning: passing argument 1 of 'x.ff' as 'float' rather than
> 'double' due to prototype
> 
> Andreas.

Sorry about the breakage.

I can reproduce this and am working on a fix.

Thanks
Dave

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] C: fix logic within c_expr::get_location
  2017-08-22 11:40   ` Andreas Schwab
  2017-08-23 16:01     ` David Malcolm
@ 2017-08-23 19:35     ` David Malcolm
  2017-08-24 11:23       ` Marek Polacek
  1 sibling, 1 reply; 7+ messages in thread
From: David Malcolm @ 2017-08-23 19:35 UTC (permalink / raw)
  To: gcc-patches, Andreas Schwab; +Cc: David Malcolm

In r251239 I added a c_expr::get_location method for use by
c_parser_expr_list for building the vec<location_t> for
an expression list, rather than using the location of the first token.

When determining whether to use the location within the tree node,
or fall back to the range in the c_expr, I used EXPR_CAN_HAVE_LOCATION,
rather than EXPR_HAS_LOCATION.  This meant that any tree nodes of kinds
that *can* have a location but which erroneously had
   EXPR_LOCATION (value) == UNKNOWN_LOCATION
had that value added to the vec<location_t>, leading to missing
location information when reporting on the issue
(seen with gcc.dg/Wtraditional-conversion-2.c for m68k).

This patch addresses this in two ways:

(a) it fixes the specific issue in this failing test case, by
    setting up the location properly on the EXCESS_PRECISION_EXPR.

(b) updating c_expr::get_location by only using the EXPR_LOCATION
    if it's sane.  It could be argued that this could be papering over
    other "missing location" bugs, but if there are any, they are
    pre-existing ones exposed by r251239, and I'd rather have this fix
    in place than play whack-a-mole on any other such bugs that may
    be lurking in the codebase.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu;
I've verified the fix with --target=m68k-elf.

OK for trunk?

gcc/c/ChangeLog:
	* c-tree.h (c_expr::get_location) Use EXPR_HAS_LOCATION rather
	than CAN_HAVE_LOCATION_P when determining whether to use the
	location_t value within "value".

gcc/c-family/ChangeLog:
	* c-lex.c (interpret_float): Use token location
	when building an EXCESS_PRECISION_EXPR.
---
 gcc/c-family/c-lex.c | 2 +-
 gcc/c/c-tree.h       | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gcc/c-family/c-lex.c b/gcc/c-family/c-lex.c
index 3765a80..a614b26 100644
--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -988,7 +988,7 @@ interpret_float (const cpp_token *token, unsigned int flags,
     }
 
   if (type != const_type)
-    value = build1 (EXCESS_PRECISION_EXPR, type, value);
+    value = build1_loc (token->src_loc, EXCESS_PRECISION_EXPR, type, value);
 
   return value;
 }
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 5182cc5..96c7ae7 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -149,7 +149,7 @@ struct c_expr
 
   location_t get_location () const
   {
-    if (CAN_HAVE_LOCATION_P (value))
+    if (EXPR_HAS_LOCATION (value))
       return EXPR_LOCATION (value);
     else
       return make_location (get_start (), get_start (), get_finish ());
-- 
1.8.5.3

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] C: fix logic within c_expr::get_location
  2017-08-23 19:35     ` [PATCH] C: fix logic within c_expr::get_location David Malcolm
@ 2017-08-24 11:23       ` Marek Polacek
  0 siblings, 0 replies; 7+ messages in thread
From: Marek Polacek @ 2017-08-24 11:23 UTC (permalink / raw)
  To: David Malcolm; +Cc: gcc-patches, Andreas Schwab

On Wed, Aug 23, 2017 at 02:08:47PM -0400, David Malcolm wrote:
> In r251239 I added a c_expr::get_location method for use by
> c_parser_expr_list for building the vec<location_t> for
> an expression list, rather than using the location of the first token.
> 
> When determining whether to use the location within the tree node,
> or fall back to the range in the c_expr, I used EXPR_CAN_HAVE_LOCATION,
> rather than EXPR_HAS_LOCATION.  This meant that any tree nodes of kinds
> that *can* have a location but which erroneously had
>    EXPR_LOCATION (value) == UNKNOWN_LOCATION
> had that value added to the vec<location_t>, leading to missing
> location information when reporting on the issue
> (seen with gcc.dg/Wtraditional-conversion-2.c for m68k).
> 
> This patch addresses this in two ways:
> 
> (a) it fixes the specific issue in this failing test case, by
>     setting up the location properly on the EXCESS_PRECISION_EXPR.
> 
> (b) updating c_expr::get_location by only using the EXPR_LOCATION
>     if it's sane.  It could be argued that this could be papering over
>     other "missing location" bugs, but if there are any, they are
>     pre-existing ones exposed by r251239, and I'd rather have this fix
>     in place than play whack-a-mole on any other such bugs that may
>     be lurking in the codebase.
> 
> Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu;
> I've verified the fix with --target=m68k-elf.
> 
> OK for trunk?

Ok, thanks.

	Marek

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-08-24 11:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-19 20:23 [PATCH 1/2] c-family/c/c++: pass optional vec<location_t> to c-format.c David Malcolm
2017-08-19 18:22 ` [PATCH 2/2] C: use full locations within c_parser_expr_list's vec<location_t> David Malcolm
2017-08-21 15:32   ` Joseph Myers
2017-08-22 11:40   ` Andreas Schwab
2017-08-23 16:01     ` David Malcolm
2017-08-23 19:35     ` [PATCH] C: fix logic within c_expr::get_location David Malcolm
2017-08-24 11:23       ` Marek Polacek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).