C: underline parameters in mismatching function calls
authorDavid Malcolm <dmalcolm@redhat.com>
Wed, 4 Oct 2017 14:10:59 +0000 (14:10 +0000)
committerDavid Malcolm <dmalcolm@gcc.gnu.org>
Wed, 4 Oct 2017 14:10:59 +0000 (14:10 +0000)
In r253096
  ("C++: underline parameters in mismatching function calls"
  aka 5d78d423a5f7a1d135c7bb678e82007678d1313c
    https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01546.html )
I updated the C++ FE's handling of mismatched types in function calls
so that it underlines the pertinent param of the callee, rather than
just the function name.

The following patch does the same for the C frontend.

Given e.g. this type mismatch:

  extern int callee (int one, const char *two, float three);

  int caller (int first, int second, float third)
  {
    return callee (first, second, third);
  }

the C FE currently emits (trunk):

  test.c: In function 'caller':
  test.c:5:25: warning: passing argument 2 of 'callee' makes pointer from
  integer without a cast [-Wint-conversion]
     return callee (first, second, third);
                           ^~~~~~
  test.c:1:12: note: expected 'const char *' but argument is of type 'int'
   extern int callee (int one, const char *two, float three);
              ^~~~~~

whereas with this patch the note underlines the pertinent param of
the callee:

  test.c: In function 'caller':
  test.c:5:25: warning: passing argument 2 of 'callee' makes pointer from
  integer without a cast [-Wint-conversion]
     return callee (first, second, third);
                           ^~~~~~
  test.c:1:41: note: expected 'const char *' but argument is of type 'int'
   extern int callee (int one, const char *two, float three);
                               ~~~~~~~~~~~~^~~

making the problem more obvious to the user.

As with the C++ patch, the patch:

(a) updates the locations of the params to cover the range of all
of their tokens, putting the caret on the first character of the
param name (if present), otherwise at the start of the first token
(doing so requires adding a last_token_location to the c_parser, so
we can determine the location of the last consumed token).

(b) updates the "note" to use the param location, rather than the
fndecl location

gcc/c/ChangeLog:
* c-decl.c (push_parm_decl): Store c_parm's location into the
PARAM_DECL.
(build_c_parm): Add "loc" param and store it within the c_parm.
* c-parser.c (struct c_parser): Add "last_token_location" field.
(c_parser_consume_token): Store location of the token into the
new field.
(c_parser_declaration_or_fndef): Store params into DECL_ARGUMENTS
when handling a FUNCTION_DECL, if it doesn't already have them.
(c_parser_parameter_declaration): Generate a location for the
parameter, and pass it to the call to build_c_parm.
* c-tree.h (struct c_parm): Add field "loc".
(build_c_parm): Add location_t param.
* c-typeck.c (get_fndecl_argument_location): New function.
(inform_for_arg): New function.
(convert_for_assignment): Use inform_for_arg when dealing with
ic_argpass.

gcc/testsuite/ChangeLog:
* gcc.dg/diagnostic-range-bad-called-object.c: Update expected
underlining for param.
* gcc.dg/param-type-mismatch.c: Update expected results to reflect
highlighting of parameters; add test coverage for trivial
parameter decls, and for callback parameters.
* gcc.dg/pr68533.c: Update location of two errors to reflect
location of params.

From-SVN: r253411

gcc/c/ChangeLog
gcc/c/c-decl.c
gcc/c/c-parser.c
gcc/c/c-tree.h
gcc/c/c-typeck.c
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.dg/diagnostic-range-bad-called-object.c
gcc/testsuite/gcc.dg/param-type-mismatch.c
gcc/testsuite/gcc.dg/pr68533.c

index 87d6c0aaca6df7044edb93b404544327471fc019..ae9d63991f062043bc26c3308de9b3b02d222b76 100644 (file)
@@ -1,3 +1,22 @@
+2017-10-04  David Malcolm  <dmalcolm@redhat.com>
+
+       * c-decl.c (push_parm_decl): Store c_parm's location into the
+       PARAM_DECL.
+       (build_c_parm): Add "loc" param and store it within the c_parm.
+       * c-parser.c (struct c_parser): Add "last_token_location" field.
+       (c_parser_consume_token): Store location of the token into the
+       new field.
+       (c_parser_declaration_or_fndef): Store params into DECL_ARGUMENTS
+       when handling a FUNCTION_DECL, if it doesn't already have them.
+       (c_parser_parameter_declaration): Generate a location for the
+       parameter, and pass it to the call to build_c_parm.
+       * c-tree.h (struct c_parm): Add field "loc".
+       (build_c_parm): Add location_t param.
+       * c-typeck.c (get_fndecl_argument_location): New function.
+       (inform_for_arg): New function.
+       (convert_for_assignment): Use inform_for_arg when dealing with
+       ic_argpass.
+
 2017-09-29  Jakub Jelinek  <jakub@redhat.com>
 
        * c-decl.c (grokfield): Use SET_DECL_C_BIT_FIELD here if
index a6b7c2041915fa90029e52bd4a618279a1e8240d..724d193f01f74a8cb47db3ec8b728603092a2f8c 100644 (file)
@@ -5190,6 +5190,8 @@ push_parm_decl (const struct c_parm *parm, tree *expr)
 
   decl = grokdeclarator (parm->declarator, parm->specs, PARM, false, NULL,
                         &attrs, expr, NULL, DEPRECATED_NORMAL);
+  if (decl && DECL_P (decl))
+    DECL_SOURCE_LOCATION (decl) = parm->loc;
   decl_attributes (&decl, attrs, 0);
 
   decl = pushdecl (decl);
@@ -9700,12 +9702,14 @@ build_void_list_node (void)
 
 struct c_parm *
 build_c_parm (struct c_declspecs *specs, tree attrs,
-             struct c_declarator *declarator)
+             struct c_declarator *declarator,
+             location_t loc)
 {
   struct c_parm *ret = XOBNEW (&parser_obstack, struct c_parm);
   ret->specs = specs;
   ret->attrs = attrs;
   ret->declarator = declarator;
+  ret->loc = loc;
   return ret;
 }
 
index a36397b7fbe63868e989e690273af5f039536c8b..1a5e39edf45bb75563f39f24b012b08895f0d56a 100644 (file)
@@ -206,6 +206,9 @@ struct GTY(()) c_parser {
   /* Buffer to hold all the tokens from parsing the vector attribute for the
      SIMD-enabled functions (formerly known as elemental functions).  */
   vec <c_token, va_gc> *cilk_simd_fn_tokens;
+
+  /* Location of the last consumed token.  */
+  location_t last_token_location;
 };
 
 /* Return a pointer to the Nth token in PARSERs tokens_buf.  */
@@ -770,6 +773,7 @@ c_parser_consume_token (c_parser *parser)
   gcc_assert (parser->tokens[0].type != CPP_EOF);
   gcc_assert (!parser->in_pragma || parser->tokens[0].type != CPP_PRAGMA_EOL);
   gcc_assert (parser->error || parser->tokens[0].type != CPP_PRAGMA);
+  parser->last_token_location = parser->tokens[0].location;
   if (parser->tokens != &parser->tokens_buf[0])
     parser->tokens++;
   else if (parser->tokens_avail == 2)
@@ -2120,6 +2124,10 @@ c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
              tree d = start_decl (declarator, specs, false,
                                   chainon (postfix_attrs,
                                            all_prefix_attrs));
+             if (d && TREE_CODE (d) == FUNCTION_DECL)
+               if (declarator->kind == cdk_function)
+                 if (DECL_ARGUMENTS (d) == NULL_TREE)
+                   DECL_ARGUMENTS (d) = declarator->u.arg_info->parms;
              if (omp_declare_simd_clauses.exists ()
                  || !vec_safe_is_empty (parser->cilk_simd_fn_tokens))
                {
@@ -4039,6 +4047,9 @@ c_parser_parameter_declaration (c_parser *parser, tree attrs)
       c_parser_skip_to_end_of_parameter (parser);
       return NULL;
     }
+
+  location_t start_loc = c_parser_peek_token (parser)->location;
+
   specs = build_null_declspecs ();
   if (attrs)
     {
@@ -4061,8 +4072,33 @@ c_parser_parameter_declaration (c_parser *parser, tree attrs)
     }
   if (c_parser_next_token_is_keyword (parser, RID_ATTRIBUTE))
     postfix_attrs = c_parser_attributes (parser);
+
+  /* Generate a location for the parameter, ranging from the start of the
+     initial token to the end of the final token.
+
+     If we have a identifier, then use it for the caret location, e.g.
+
+       extern int callee (int one, int (*two)(int, int), float three);
+                                   ~~~~~~^~~~~~~~~~~~~~
+
+     otherwise, reuse the start location for the caret location e.g.:
+
+       extern int callee (int one, int (*)(int, int), float three);
+                                   ^~~~~~~~~~~~~~~~~
+  */
+  location_t end_loc = parser->last_token_location;
+
+  /* Find any cdk_id declarator; determine if we have an identifier.  */
+  c_declarator *id_declarator = declarator;
+  while (id_declarator && id_declarator->kind != cdk_id)
+    id_declarator = id_declarator->declarator;
+  location_t caret_loc = (id_declarator->u.id
+                         ? id_declarator->id_loc
+                         : start_loc);
+  location_t param_loc = make_location (caret_loc, start_loc, end_loc);
+
   return build_c_parm (specs, chainon (postfix_attrs, prefix_attrs),
-                      declarator);
+                      declarator, param_loc);
 }
 
 /* Parse a string literal in an asm expression.  It should not be
index 96c7ae7613ff5049b2d39bb19266ea2706de8193..11356476d840b7f8b95351bef82be01d85da86a0 100644 (file)
@@ -477,6 +477,8 @@ struct c_parm {
   tree attrs;
   /* The declarator.  */
   struct c_declarator *declarator;
+  /* The location of the parameter.  */
+  location_t loc;
 };
 
 /* Used when parsing an enum.  Initialized by start_enum.  */
@@ -581,7 +583,7 @@ extern void temp_pop_parm_decls (void);
 extern tree xref_tag (enum tree_code, tree);
 extern struct c_typespec parser_xref_tag (location_t, enum tree_code, tree);
 extern struct c_parm *build_c_parm (struct c_declspecs *, tree,
-                                   struct c_declarator *);
+                                   struct c_declarator *, location_t);
 extern struct c_declarator *build_attrs_declarator (tree,
                                                    struct c_declarator *);
 extern struct c_declarator *build_function_declarator (struct c_arg_info *,
index 73e74602f595b0a581ff7ed34f8aceff4cbe3518..2a10813190e527ccd5b0932616526cdb2b5ba815 100644 (file)
@@ -6180,6 +6180,50 @@ maybe_warn_string_init (location_t loc, tree type, struct c_expr expr)
                  "array initialized from parenthesized string constant");
 }
 
+/* Attempt to locate the parameter with the given index within FNDECL,
+   returning DECL_SOURCE_LOCATION (FNDECL) if it can't be found.  */
+
+static location_t
+get_fndecl_argument_location (tree fndecl, int argnum)
+{
+  int i;
+  tree param;
+
+  /* Locate param by index within DECL_ARGUMENTS (fndecl).  */
+  for (i = 0, param = DECL_ARGUMENTS (fndecl);
+       i < argnum && param;
+       i++, param = TREE_CHAIN (param))
+    ;
+
+  /* If something went wrong (e.g. if we have a builtin and thus no arguments),
+     return DECL_SOURCE_LOCATION (FNDECL).  */
+  if (param == NULL)
+    return DECL_SOURCE_LOCATION (fndecl);
+
+  return DECL_SOURCE_LOCATION (param);
+}
+
+/* Issue a note about a mismatching argument for parameter PARMNUM
+   to FUNDECL, for types EXPECTED_TYPE and ACTUAL_TYPE.
+   Attempt to issue the note at the pertinent parameter of the decl;
+   failing that issue it at the location of FUNDECL; failing that
+   issue it at PLOC.  */
+
+static void
+inform_for_arg (tree fundecl, location_t ploc, int parmnum,
+               tree expected_type, tree actual_type)
+{
+  location_t loc;
+  if (fundecl && !DECL_IS_BUILTIN (fundecl))
+    loc = get_fndecl_argument_location (fundecl, parmnum - 1);
+  else
+    loc = ploc;
+
+  inform (loc,
+         "expected %qT but argument is of type %qT",
+         expected_type, actual_type);
+}
+
 /* Convert value RHS to type TYPE as preparation for an assignment to
    an lvalue of type TYPE.  If ORIGTYPE is not NULL_TREE, it is the
    original type of RHS; this differs from TREE_TYPE (RHS) for enum
@@ -6251,10 +6295,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
       {                                                                  \
       case ic_argpass:                                                   \
         if (pedwarn (PLOC, OPT, AR, parmnum, rname))                    \
-          inform ((fundecl && !DECL_IS_BUILTIN (fundecl))               \
-                 ? DECL_SOURCE_LOCATION (fundecl) : PLOC,               \
-                  "expected %qT but argument is of type %qT",            \
-                  type, rhstype);                                        \
+          inform_for_arg (fundecl, (PLOC), parmnum, type, rhstype);    \
         break;                                                           \
       case ic_assign:                                                    \
         pedwarn (LOCATION, OPT, AS);                                     \
@@ -6280,10 +6321,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
       {                                                                  \
       case ic_argpass:                                                   \
         if (pedwarn (PLOC, OPT, AR, parmnum, rname, QUALS))             \
-          inform ((fundecl && !DECL_IS_BUILTIN (fundecl))               \
-                 ? DECL_SOURCE_LOCATION (fundecl) : PLOC,               \
-                  "expected %qT but argument is of type %qT",            \
-                  type, rhstype);                                        \
+          inform_for_arg (fundecl, (PLOC), parmnum, type, rhstype);    \
         break;                                                           \
       case ic_assign:                                                    \
         pedwarn (LOCATION, OPT, AS, QUALS);                             \
@@ -6309,10 +6347,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
       {                                                                  \
       case ic_argpass:                                                   \
         if (warning_at (PLOC, OPT, AR, parmnum, rname, QUALS))           \
-          inform ((fundecl && !DECL_IS_BUILTIN (fundecl))                \
-                  ? DECL_SOURCE_LOCATION (fundecl) : PLOC,               \
-                  "expected %qT but argument is of type %qT",            \
-                  type, rhstype);                                        \
+          inform_for_arg (fundecl, (PLOC), parmnum, type, rhstype);      \
         break;                                                           \
       case ic_assign:                                                    \
         warning_at (LOCATION, OPT, AS, QUALS);                           \
@@ -6864,10 +6899,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
              if (pedwarn (expr_loc, OPT_Wincompatible_pointer_types,
                           "passing argument %d of %qE from incompatible "
                           "pointer type", parmnum, rname))
-               inform ((fundecl && !DECL_IS_BUILTIN (fundecl))
-                       ? DECL_SOURCE_LOCATION (fundecl) : expr_loc,
-                       "expected %qT but argument is of type %qT",
-                       type, rhstype);
+               inform_for_arg (fundecl, expr_loc, parmnum, type, rhstype);
              break;
            case ic_assign:
              pedwarn (location, OPT_Wincompatible_pointer_types,
@@ -6910,10 +6942,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
            if (pedwarn (expr_loc, OPT_Wint_conversion,
                         "passing argument %d of %qE makes pointer from "
                         "integer without a cast", parmnum, rname))
-             inform ((fundecl && !DECL_IS_BUILTIN (fundecl))
-                     ? DECL_SOURCE_LOCATION (fundecl) : expr_loc,
-                     "expected %qT but argument is of type %qT",
-                     type, rhstype);
+             inform_for_arg (fundecl, expr_loc, parmnum, type, rhstype);
            break;
          case ic_assign:
            pedwarn (location, OPT_Wint_conversion,
@@ -6944,10 +6973,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
          if (pedwarn (expr_loc, OPT_Wint_conversion,
                       "passing argument %d of %qE makes integer from "
                       "pointer without a cast", parmnum, rname))
-           inform ((fundecl && !DECL_IS_BUILTIN (fundecl))
-                   ? DECL_SOURCE_LOCATION (fundecl) : expr_loc,
-                   "expected %qT but argument is of type %qT",
-                   type, rhstype);
+           inform_for_arg (fundecl, expr_loc, parmnum, type, rhstype);
          break;
        case ic_assign:
          pedwarn (location, OPT_Wint_conversion,
@@ -6985,9 +7011,7 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
     case ic_argpass:
       error_at (expr_loc, "incompatible type for argument %d of %qE", parmnum,
                rname);
-      inform ((fundecl && !DECL_IS_BUILTIN (fundecl))
-             ? DECL_SOURCE_LOCATION (fundecl) : expr_loc,
-             "expected %qT but argument is of type %qT", type, rhstype);
+      inform_for_arg (fundecl, expr_loc, parmnum, type, rhstype);
       break;
     case ic_assign:
       error_at (location, "incompatible types when assigning to type %qT from "
index 61357e87f20316b4dce799e1f13717511355c9d5..eb9f5b88fe862ae1d9b57fc701a5ace032a09bf9 100644 (file)
@@ -1,3 +1,13 @@
+2017-10-04  David Malcolm  <dmalcolm@redhat.com>
+
+       * gcc.dg/diagnostic-range-bad-called-object.c: Update expected
+       underlining for param.
+       * gcc.dg/param-type-mismatch.c: Update expected results to reflect
+       highlighting of parameters; add test coverage for trivial
+       parameter decls, and for callback parameters.
+       * gcc.dg/pr68533.c: Update location of two errors to reflect
+       location of params.
+
 2017-10-04  David Malcolm  <dmalcolm@redhat.com>
 
        * jit.dg/test-expressions.c (make_test_of_vectors): New function.
index 95fb3e9ec5acc6c406837d192b69885ffb61302a..0f7fd2a28569584fa60796c254a506450b2bedb8 100644 (file)
@@ -19,6 +19,6 @@ void call_of_non_function_ptr (char **argP, char **argQ)
    { dg-end-multiline-output "" }
    { dg-begin-multiline-output "" }
  void call_of_non_function_ptr (char **argP, char **argQ)
-                                       ^~~~
+                                ~~~~~~~^~~~
    { dg-end-multiline-output "" } */
 }
index 70ea0bc268bfb83096bb3a478ca391a4ebfd5efe..9498a748f308ed8e38ac2f7d38a3a8c18b075c17 100644 (file)
@@ -1,9 +1,6 @@
 /* { dg-options "-fdiagnostics-show-caret" }  */
 
-/* A collection of calls where argument 2 is of the wrong type.
-
-   TODO: we should highlight the second parameter of the callee, rather
-   than its name.  */
+/* A collection of calls where argument 2 is of the wrong type.  */
 
 /* decl, with argname.  */
 
@@ -19,7 +16,7 @@ int test_1 (int first, int second, float third)
   /* { dg-message "expected 'const char \\*' but argument is of type 'int'" "" { target *-*-* } callee_1 } */
   /* { dg-begin-multiline-output "" }
  extern int callee_1 (int one, const char *two, float three);
-            ^~~~~~~~
+                               ~~~~~~~~~~~~^~~
      { dg-end-multiline-output "" } */
 }
 
@@ -37,7 +34,7 @@ int test_2 (int first, int second, float third)
   /* { dg-message "expected 'const char \\*' but argument is of type 'int'" "" { target *-*-* } callee_2 } */
   /* { dg-begin-multiline-output "" }
  extern int callee_2 (int, const char *, float);
-            ^~~~~~~~
+                           ^~~~~~~~~~~~
      { dg-end-multiline-output "" } */
 }
 
@@ -58,6 +55,78 @@ int test_3 (int first, int second, float third)
   /* { dg-message "expected 'const char \\*' but argument is of type 'int'" "" { target *-*-* } callee_3 } */
   /* { dg-begin-multiline-output "" }
  static int callee_3 (int one, const char *two, float three)
-            ^~~~~~~~
+                               ~~~~~~~~~~~~^~~
+     { dg-end-multiline-output "" } */
+}
+
+/* Trivial decl, with argname.  */
+
+extern int callee_4 (int one, float two, float three); /* { dg-line callee_4 } */
+
+int test_4 (int first, const char *second, float third)
+{
+  return callee_4 (first, second, third); /* { dg-error "incompatible type for argument 2 of 'callee_4'" }  */
+  /* { dg-begin-multiline-output "" }
+   return callee_4 (first, second, third);
+                           ^~~~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-message "expected 'float' but argument is of type 'const char \\*'" "" { target *-*-* } callee_4 } */
+  /* { dg-begin-multiline-output "" }
+ extern int callee_4 (int one, float two, float three);
+                               ~~~~~~^~~
+     { dg-end-multiline-output "" } */
+}
+
+/* Trivial decl, without argname.  */
+
+extern int callee_5 (int, float, float); /* { dg-line callee_5 } */
+
+int test_5 (int first, const char *second, float third)
+{
+  return callee_5 (first, second, third); /* { dg-error "incompatible type for argument 2 of 'callee_5'" }  */
+  /* { dg-begin-multiline-output "" }
+   return callee_5 (first, second, third);
+                           ^~~~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-message "expected 'float' but argument is of type 'const char \\*'" "" { target *-*-* } callee_5 } */
+  /* { dg-begin-multiline-output "" }
+ extern int callee_5 (int, float, float);
+                           ^~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* Callback with name.  */
+
+extern int callee_6 (int one, int (*two)(int, int), float three); /* { dg-line callee_6 } */
+
+int test_6 (int first, int second, float third)
+{
+  return callee_6 (first, second, third); /* { dg-warning "passing argument 2 of 'callee_6' makes pointer from integer without a cast" } */
+  /* { dg-begin-multiline-output "" }
+   return callee_6 (first, second, third);
+                           ^~~~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-message " expected 'int \\(\\*\\)\\(int,  int\\)' but argument is of type 'int'" "" { target *-*-* } callee_6 } */
+  /* { dg-begin-multiline-output "" }
+ extern int callee_6 (int one, int (*two)(int, int), float three);
+                               ~~~~~~^~~~~~~~~~~~~~
+     { dg-end-multiline-output "" } */
+}
+
+/* Callback without name.  */
+
+extern int callee_7 (int one, int (*)(int, int), float three); /* { dg-line callee_7 } */
+
+int test_7 (int first, int second, float third)
+{
+  return callee_7 (first, second, third); /* { dg-warning "passing argument 2 of 'callee_7' makes pointer from integer without a cast" } */
+  /* { dg-begin-multiline-output "" }
+   return callee_7 (first, second, third);
+                           ^~~~~~
+     { dg-end-multiline-output "" } */
+  /* { dg-message " expected 'int \\(\\*\\)\\(int,  int\\)' but argument is of type 'int'" "" { target *-*-* } callee_7 } */
+  /* { dg-begin-multiline-output "" }
+ extern int callee_7 (int one, int (*)(int, int), float three);
+                               ^~~~~~~~~~~~~~~~~
      { dg-end-multiline-output "" } */
 }
index e1a1f31f65ae571f75bba81b6ac8bd7e3478991d..49e67a96168ec99a3bbb324ed7ee1c2dd3682f8f 100644 (file)
@@ -28,8 +28,8 @@ f2 (
 
 void
 f3 (
-  const void
-   )           /* { dg-error "'void' as only parameter may not be qualified" } */
+  const void   /* { dg-error "'void' as only parameter may not be qualified" } */
+   )
 {
 }
 
@@ -54,8 +54,8 @@ void
 f6 (
    int
    x,
-   void
-   )           /* { dg-error "'void' must be the only parameter" } */
+   void        /* { dg-error "'void' must be the only parameter" } */
+   )
 {
 }