[PATCH] Use positional_argument in callback_only attribute handler

Hi,
this is what I came up with.  I couldn't eliminate all the manual
checking, as positional_argument doesn't check for nested types for
example, but I tried to use it wherever I could.

Regtested with the rest of the series without problems.  OK for master?

Best regards,
Josef

-- >8 --

This patch replaces some of the manual bounds checking done in the
callback_only attribute handler with calls to positional_argument.
Since positional_argument issues warnings and the attribute handler
issues errors, I decided to downgrade the errors to warnings for
consistency.

gcc/c-family/ChangeLog:

	* c-attribs.cc (handle_callback_only_attribute): Use
	positional_argument for bounds checking in callback_only
	attribute handler, downgrade errors to warnings for
	consistency.

gcc/testsuite/ChangeLog:

	* gcc.dg/attr-callback.c: Expect warnings instead of errors,
	adjust expected messages.

Signed-off-by: Josef Melcr <[email protected]>
---
 gcc/c-family/c-attribs.cc            | 103 ++++++++++++---------------
 gcc/testsuite/gcc.dg/attr-callback.c |  46 ++++++++----
 2 files changed, 77 insertions(+), 72 deletions(-)

diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
index 1ed02850a91..01150136a55 100644
--- a/gcc/c-family/c-attribs.cc
+++ b/gcc/c-family/c-attribs.cc
@@ -4682,66 +4682,52 @@ handle_callback_only_attribute (tree *node, tree name, tree args,
   tree decl = *node;
   if (TREE_CODE (decl) != FUNCTION_DECL)
     {
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"%qE attribute can only be used on functions", name);
+      warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+		  "%qE attribute can only be used on functions", name);
       *no_add_attrs = true;
     }
 
-  tree cb_fn_idx_node = TREE_VALUE (args);
-  if (TREE_CODE (cb_fn_idx_node) != INTEGER_CST)
+  tree val = positional_argument (decl, name, TREE_VALUE (args), POINTER_TYPE,
+				  1, POSARG_ZERO);
+  if (!val)
     {
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"argument specifying callback function position is not an "
-		"integer constant");
       *no_add_attrs = true;
       return NULL_TREE;
     }
+  TREE_VALUE (args) = val;
+
   /* We have to use the function type for validation, as
      DECL_ARGUMENTS returns NULL at this point.  */
-  int callback_fn_idx = TREE_INT_CST_LOW (cb_fn_idx_node);
+  int callback_fn_idx = TREE_INT_CST_LOW (val);
   tree decl_type_args = TYPE_ARG_TYPES (TREE_TYPE (decl));
   tree it;
-  int decl_nargs = list_length (decl_type_args);
   for (it = decl_type_args; it != NULL_TREE; it = TREE_CHAIN (it))
     if (it == void_list_node)
-      {
-	--decl_nargs;
-	break;
-      }
+      break;
+
   if (callback_fn_idx == CB_UNKNOWN_POS)
     {
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"callback function position cannot be marked as unknown");
+      warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+		  "callback function position cannot be marked as unknown");
       *no_add_attrs = true;
       return NULL_TREE;
     }
+
   --callback_fn_idx;
-  if (callback_fn_idx >= decl_nargs)
-    {
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"callback function position out of range");
-      *no_add_attrs = true;
-      return NULL_TREE;
-    }
 
-  /* Search for the type of the callback function
-     in parameters of the original function.  */
+  /* Search for the type of the callback function in parameters of the original
+     function.  We know it's there because it's been validated by
+     positional_argument.  */
   tree cfn = chain_index (callback_fn_idx, decl_type_args);
-  if (cfn == NULL_TREE)
-    {
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"could not retrieve callback function from arguments");
-      *no_add_attrs = true;
-      return NULL_TREE;
-    }
+  gcc_checking_assert (cfn != NULL_TREE);
   cfn = TREE_VALUE (cfn);
   tree cfn_pointee_type = TREE_TYPE (cfn);
   if (TREE_CODE (cfn) != POINTER_TYPE
       || TREE_CODE (cfn_pointee_type) != FUNCTION_TYPE)
     {
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"argument no. %d is not an address of a function",
-		callback_fn_idx + 1);
+      warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+		  "argument no. %d is not an address of a function",
+		  callback_fn_idx + 1);
       *no_add_attrs = true;
       return NULL_TREE;
     }
@@ -4759,9 +4745,9 @@ handle_callback_only_attribute (tree *node, tree name, tree args,
       }
   if (cfn_nargs != type_nargs)
     {
-      error_at (DECL_SOURCE_LOCATION (decl),
-		"argument number mismatch, %d expected, got %d", type_nargs,
-		cfn_nargs);
+      warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+		  "argument number mismatch, %d expected, got %d", type_nargs,
+		  cfn_nargs);
       *no_add_attrs = true;
       return NULL_TREE;
     }
@@ -4777,12 +4763,23 @@ handle_callback_only_attribute (tree *node, tree name, tree args,
     {
       if (TREE_CODE (TREE_VALUE (cfn_it)) != INTEGER_CST)
 	{
-	  error_at (DECL_SOURCE_LOCATION (decl),
-		    "argument no. %d is not an integer constant", curr + 1);
+	  warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+		      "argument no. %d is not an integer constant", curr + 1);
 	  *no_add_attrs = true;
 	  continue;
 	}
 
+      tree expected_type = TREE_VALUE (it);
+      tree arg_val = positional_argument (decl, name, TREE_VALUE (cfn_it),
+					  TREE_CODE (expected_type), curr + 1,
+					  POSARG_ZERO);
+      if (!arg_val)
+	{
+	  *no_add_attrs = true;
+	  return NULL_TREE;
+	}
+
+      TREE_VALUE (cfn_it) = arg_val;
       int arg_idx = TREE_INT_CST_LOW (TREE_VALUE (cfn_it));
 
       /* No need to check for type compatibility,
@@ -4791,29 +4788,19 @@ handle_callback_only_attribute (tree *node, tree name, tree args,
 	continue;
 
       arg_idx -= 1;
-      /* Report an error if the position is out of bounds,
-	 but we can still check the rest of the arguments.  */
-      if (arg_idx >= decl_nargs)
-	{
-	  error_at (DECL_SOURCE_LOCATION (decl),
-		    "callback argument index %d is out of range", arg_idx + 1);
-	  *no_add_attrs = true;
-	  continue;
-	}
-
       tree arg_type = chain_index (arg_idx, decl_type_args);
       gcc_checking_assert (arg_type != NULL_TREE);
       arg_type = TREE_VALUE (arg_type);
-      tree expected_type = TREE_VALUE (it);
       /* Check the type of the value we are about to pass ("arg_type")
 	 for compatibility with the actual type the callback function
 	 expects ("expected_type").  */
       if (!types_compatible_p (expected_type, arg_type))
 	{
-	  error_at (DECL_SOURCE_LOCATION (decl),
-		    "argument type at index %d is not compatible with callback "
-		    "argument type at index %d",
-		    arg_idx + 1, curr + 1);
+	  warning_at (
+	    DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+	    "argument type at index %d is not compatible with callback "
+	    "argument type at index %d",
+	    arg_idx + 1, curr + 1);
 	  *no_add_attrs = true;
 	  continue;
 	}
@@ -4825,10 +4812,10 @@ handle_callback_only_attribute (tree *node, tree name, tree args,
   for (; it; it = lookup_attribute ("callback_only", TREE_CHAIN (it)))
     if (callback_get_fn_index (it) == callback_fn_idx)
       {
-	error_at (DECL_SOURCE_LOCATION (decl),
-		  "function declaration has multiple callback attributes "
-		  "describing argument no. %d",
-		  callback_fn_idx + 1);
+	warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
+		    "function declaration has multiple callback attributes "
+		    "describing argument no. %d",
+		    callback_fn_idx + 1);
 	*no_add_attrs = true;
 	break;
       }
diff --git a/gcc/testsuite/gcc.dg/attr-callback.c b/gcc/testsuite/gcc.dg/attr-callback.c
index d9cc67c3aff..74c8287471b 100755
--- a/gcc/testsuite/gcc.dg/attr-callback.c
+++ b/gcc/testsuite/gcc.dg/attr-callback.c
@@ -25,15 +25,15 @@ unknown_2(void (*)(int*, double*), int*, double*, char*);
 
 [[gnu::callback_only(1, 0, 3, 3)]]
 void
-too_many(void (*)(int*, double*), int*, double*); /* { dg-error "argument number mismatch, 2 expected, got 3" }*/
+too_many(void (*)(int*, double*), int*, double*); /* { dg-warning "argument number mismatch, 2 expected, got 3" }*/
 
 [[gnu::callback_only(1, 2)]]
 void
-too_few_1(void (*)(int*, double*), int*, double*); /* { dg-error "argument number mismatch, 2 expected, got 1" }*/
+too_few_1(void (*)(int*, double*), int*, double*); /* { dg-warning "argument number mismatch, 2 expected, got 1" }*/
 
 [[gnu::callback_only(1)]]
 void
-too_few_2(void (*)(int*, double*), int*, double*); /* { dg-error "argument number mismatch, 2 expected, got 0" }*/
+too_few_2(void (*)(int*, double*), int*, double*); /* { dg-warning "argument number mismatch, 2 expected, got 0" }*/
 
 [[gnu::callback_only(3, 1)]]
 void
@@ -45,52 +45,70 @@ downcast(char*, void* (*)(float*), double*);
 
 [[gnu::callback_only(1, 2, 5)]]
 void
-out_of_range_1(char (*)(float*, double*), float*, double*, int*); /* { dg-error "callback argument index 5 is out of range" } */
+out_of_range_1(char (*)(float*, double*), float*, double*, int*); /* { dg-warning "exceeds the number of function parameters" } */
 
 [[gnu::callback_only(1, -2, 3)]]
 void
-out_of_range_2(char (*)(float*, double*), float*, double*, int*); /* { dg-error "callback argument index -2 is out of range" } */
+out_of_range_2(char (*)(float*, double*), float*, double*, int*); /* { dg-warning "exceeds the number of function parameters" } */
 
 [[gnu::callback_only(-1, 2, 3)]]
 void
-out_of_range_3(char (*)(float*, double*), float*, double*, int*); /* { dg-error "callback function index -1 is out of range" } */
+out_of_range_3(char (*)(float*, double*), float*, double*, int*);  /* { dg-warning "exceeds the number of function parameters" } */
 
 [[gnu::callback_only(67, 2, 3)]]
 void
-out_of_range_4(char (*)(float*, double*), float*, double*, int*); /* { dg-error "callback function index 67 is out of range" } */
+out_of_range_4(char (*)(float*, double*), float*, double*, int*); /* { dg-warning "exceeds the number of function parameters" } */
 
 [[gnu::callback_only(0, 2, 3)]]
 void
-unknown_fn(char (*)(float*, double*), float*, double*, int*); /* { dg-error "callback function position cannot be marked as unknown" } */
+unknown_fn(char (*)(float*, double*), float*, double*, int*); /* { dg-warning "callback function position cannot be marked as unknown" } */
 
 [[gnu::callback_only(1, 2)]]
 void
-not_a_fn(int, int); /* { dg-error "argument no. 1 is not an address of a function" } */
+not_a_fn(int, int); /* { dg-warning "refers to" } */
 
 struct S
 {
   int x;
 };
 
+static struct S placeholder;
+
+static int one = 1;
+
+static const int const_one = 1;
+
 [[gnu::callback_only(1, 2)]]
 void
-incompatible_types_1(void (*)(struct S*), struct S); /* { dg-error "argument type at index 2 is not compatible with callback argument type at index 1" } */
+incompatible_types_1(void (*)(struct S*), struct S); /* { dg-warning "refers to" } */
 
 [[gnu::callback_only(1, 3, 2)]]
 void
-incompatible_types_2(void (*)(struct S*, int*), int*, double); /* { dg-error "argument type at index 3 is not compatible with callback argument type at index 1" } */
+incompatible_types_2(void (*)(struct S*, int*), int*, double); /* { dg-warning "refers to" } */
 
 [[gnu::callback_only(1, "2")]]
 void
-wrong_arg_type_1(void (*)(void*), void*); /* { dg-error "argument no. 1 is not an integer constant" } */
+wrong_arg_type_1(void (*)(void*), void*); /* { dg-warning "argument no. 1 is not an integer constant" } */
 
 [[gnu::callback_only("not a number", 2, 2)]]
 void
-wrong_arg_type_2(void (*)(void*, void*), void*); /* { dg-error "argument specifying callback function position is not an integer constant" } */
+wrong_arg_type_2(void (*)(void*, void*), void*); /* { dg-warning "has type" } */
+
+[[gnu::callback_only(placeholder, 2, 2)]]
+void
+wrong_arg_type_3(void (*)(void*, void*), void*); /* { dg-warning "has type" } */
+
+[[gnu::callback_only(one, 2, 2)]]
+void
+int_identifier(void (*)(void*, void*), void*); /* { dg-warning "is not an integer constant" } */
+
+[[gnu::callback_only(one, 2, 2)]]
+void
+int_identifier_1(void (*)(void*, void*), void*); /* { dg-warning "is not an integer constant" } */
 
 [[gnu::callback_only(1, 2), gnu::callback_only(1, 3)]]
 void
-multiple_single_fn(void (*)(int*), int*, int*); /* { dg-error "function declaration has multiple callback attributes describing argument no. 1" } */
+multiple_single_fn(void (*)(int*), int*, int*); /* { dg-warning "function declaration has multiple callback attributes describing argument no. 1" } */
 
 /* Check that the attribute won't resolve outside of our namespace.  */
 
-- 
2.55.0