diff mbox

[1/2] Make make-target-delegates grok namespace scope op and template params

Message ID 1509377531-21044-2-git-send-email-palves@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves Oct. 30, 2017, 3:32 p.m. UTC 
  The next patch will want to use gdb::array_view<int> as parameter type
of a target_ops method.  However, that runs into a
make-target-delegates limitation: target_debug_foo calls in
target-delegates.c for parameters/return types with namespace scope
operators ("::") or template parameters, end up looking like:

 @@ -1313,9 +1313,7 @@ debug_set_syscall_catchpoint (struct target_ops *self, int arg1, int arg2, int a
    fputs_unfiltered (", ", gdb_stdlog);
    target_debug_print_int (arg3);
    fputs_unfiltered (", ", gdb_stdlog);
 -  target_debug_print_int (arg4);
 -  fputs_unfiltered (", ", gdb_stdlog);
 -  target_debug_print_int_p (arg5);
 +  target_debug_print_gdb::array_view<const_int> (arg4);

which obviously isn't something that compiles.  The problem is that
make-target-delegates wasn't ever taught that '::', '<', and '>' can
appear in parameter/return types.  You could work around it by hidding
the unsupported characters behind a typedef in the target method
declaration, or by using an explicit TARGET_DEBUG_PRINTER, but it's
better to just remove the limitation.

While at it, also fix an "abuse" of reserved identifiers.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* make-target-delegates (munge_type): Also munge '<', '>', and
	':'.  Avoid double underscores in identifiers, and trailing
	underscores.
	* target-debug.h
	(target_debug_print_VEC_static_tracepoint_marker_p__p): Rename to
	...
	(target_debug_print_VEC_static_tracepoint_marker_p_p): ... this.
	* target-delegates.c: Regenerate.
---
 gdb/make-target-delegates | 12 +++++++++++-
 gdb/target-debug.h        |  2 +-
 gdb/target-delegates.c    |  2 +-
 3 files changed, 13 insertions(+), 3 deletions(-)

Comments

Simon Marchi Oct. 30, 2017, 4:02 p.m. UTC | #1 
On 2017-10-30 11:32, Pedro Alves wrote:
> The next patch will want to use gdb::array_view<int> as parameter type
> of a target_ops method.  However, that runs into a
> make-target-delegates limitation: target_debug_foo calls in
> target-delegates.c for parameters/return types with namespace scope
> operators ("::") or template parameters, end up looking like:
> 
>  @@ -1313,9 +1313,7 @@ debug_set_syscall_catchpoint (struct target_ops
> *self, int arg1, int arg2, int a
>     fputs_unfiltered (", ", gdb_stdlog);
>     target_debug_print_int (arg3);
>     fputs_unfiltered (", ", gdb_stdlog);
>  -  target_debug_print_int (arg4);
>  -  fputs_unfiltered (", ", gdb_stdlog);
>  -  target_debug_print_int_p (arg5);
>  +  target_debug_print_gdb::array_view<const_int> (arg4);
> 
> which obviously isn't something that compiles.  The problem is that
> make-target-delegates wasn't ever taught that '::', '<', and '>' can
> appear in parameter/return types.  You could work around it by hidding
> the unsupported characters behind a typedef in the target method
> declaration, or by using an explicit TARGET_DEBUG_PRINTER, but it's
> better to just remove the limitation.
> 
> While at it, also fix an "abuse" of reserved identifiers.
> 
> gdb/ChangeLog:
> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
> 
> 	* make-target-delegates (munge_type): Also munge '<', '>', and
> 	':'.  Avoid double underscores in identifiers, and trailing
> 	underscores.
> 	* target-debug.h
> 	(target_debug_print_VEC_static_tracepoint_marker_p__p): Rename to
> 	...
> 	(target_debug_print_VEC_static_tracepoint_marker_p_p): ... this.
> 	* target-delegates.c: Regenerate.
> ---
>  gdb/make-target-delegates | 12 +++++++++++-
>  gdb/target-debug.h        |  2 +-
>  gdb/target-delegates.c    |  2 +-
>  3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/make-target-delegates b/gdb/make-target-delegates
> index fd51c64..1773232 100755
> --- a/gdb/make-target-delegates
> +++ b/gdb/make-target-delegates
> @@ -232,8 +232,18 @@ sub munge_type($) {
>  	$result = $1;
>      } else {
>  	($result = $typename) =~ s/\s+$//;
> -	$result =~ s/[ ()]/_/g;
> +	$result =~ s/[ ()<>:]/_/g;
>  	$result =~ s/[*]/p/g;
> +
> +	# Identifers with double underscores are reserved to the C++
> +	# implementation.
> +	$result =~ s/_+/_/g;
> +
> +	# Avoid ending the function name with underscore, for
> +	# cosmetics.  Trailing underscores appear after munging types
> +	# with template parameters, like e.g. "foo<int>".
> +	$result =~ s/_$//g;
> +
>  	$result = 'target_debug_print_' . $result;
>      }
> 
> diff --git a/gdb/target-debug.h b/gdb/target-debug.h
> index 14196b4..068495e 100644
> --- a/gdb/target-debug.h
> +++ b/gdb/target-debug.h
> @@ -116,7 +116,7 @@
>    target_debug_do_print (host_address_to_string (X))
>  #define target_debug_print_mem_region_vector(X) \
>    target_debug_do_print (host_address_to_string (X.data ()))
> -#define target_debug_print_VEC_static_tracepoint_marker_p__p(X)	\
> +#define target_debug_print_VEC_static_tracepoint_marker_p_p(X)	\
>    target_debug_do_print (host_address_to_string (X))
>  #define target_debug_print_const_struct_target_desc_p(X)	\
>    target_debug_do_print (host_address_to_string (X))
> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
> index e0d7a9a..1cbe6f8 100644
> --- a/gdb/target-delegates.c
> +++ b/gdb/target-delegates.c
> @@ -3352,7 +3352,7 @@ debug_static_tracepoint_markers_by_strid (struct
> target_ops *self, const char *a
>    fputs_unfiltered (", ", gdb_stdlog);
>    target_debug_print_const_char_p (arg1);
>    fputs_unfiltered (") = ", gdb_stdlog);
> -  target_debug_print_VEC_static_tracepoint_marker_p__p (result);
> +  target_debug_print_VEC_static_tracepoint_marker_p_p (result);
>    fputs_unfiltered ("\n", gdb_stdlog);
>    return result;
>  }

LGTM, I'll make a patch to remove the mem_region_vector typedef.

Simon
Simon Marchi Dec. 3, 2017, 12:51 a.m. UTC | #2 
On 2017-10-30 12:02 PM, Simon Marchi wrote:
> On 2017-10-30 11:32, Pedro Alves wrote:
>> The next patch will want to use gdb::array_view<int> as parameter type
>> of a target_ops method.  However, that runs into a
>> make-target-delegates limitation: target_debug_foo calls in
>> target-delegates.c for parameters/return types with namespace scope
>> operators ("::") or template parameters, end up looking like:
>>
>>  @@ -1313,9 +1313,7 @@ debug_set_syscall_catchpoint (struct target_ops
>> *self, int arg1, int arg2, int a
>>     fputs_unfiltered (", ", gdb_stdlog);
>>     target_debug_print_int (arg3);
>>     fputs_unfiltered (", ", gdb_stdlog);
>>  -  target_debug_print_int (arg4);
>>  -  fputs_unfiltered (", ", gdb_stdlog);
>>  -  target_debug_print_int_p (arg5);
>>  +  target_debug_print_gdb::array_view<const_int> (arg4);
>>
>> which obviously isn't something that compiles.  The problem is that
>> make-target-delegates wasn't ever taught that '::', '<', and '>' can
>> appear in parameter/return types.  You could work around it by hidding
>> the unsupported characters behind a typedef in the target method
>> declaration, or by using an explicit TARGET_DEBUG_PRINTER, but it's
>> better to just remove the limitation.
>>
>> While at it, also fix an "abuse" of reserved identifiers.
>>
>> gdb/ChangeLog:
>> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>
>>
>> 	* make-target-delegates (munge_type): Also munge '<', '>', and
>> 	':'.  Avoid double underscores in identifiers, and trailing
>> 	underscores.
>> 	* target-debug.h
>> 	(target_debug_print_VEC_static_tracepoint_marker_p__p): Rename to
>> 	...
>> 	(target_debug_print_VEC_static_tracepoint_marker_p_p): ... this.
>> 	* target-delegates.c: Regenerate.
>> ---
>>  gdb/make-target-delegates | 12 +++++++++++-
>>  gdb/target-debug.h        |  2 +-
>>  gdb/target-delegates.c    |  2 +-
>>  3 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/make-target-delegates b/gdb/make-target-delegates
>> index fd51c64..1773232 100755
>> --- a/gdb/make-target-delegates
>> +++ b/gdb/make-target-delegates
>> @@ -232,8 +232,18 @@ sub munge_type($) {
>>  	$result = $1;
>>      } else {
>>  	($result = $typename) =~ s/\s+$//;
>> -	$result =~ s/[ ()]/_/g;
>> +	$result =~ s/[ ()<>:]/_/g;
>>  	$result =~ s/[*]/p/g;
>> +
>> +	# Identifers with double underscores are reserved to the C++
>> +	# implementation.
>> +	$result =~ s/_+/_/g;
>> +
>> +	# Avoid ending the function name with underscore, for
>> +	# cosmetics.  Trailing underscores appear after munging types
>> +	# with template parameters, like e.g. "foo<int>".
>> +	$result =~ s/_$//g;
>> +
>>  	$result = 'target_debug_print_' . $result;
>>      }
>>
>> diff --git a/gdb/target-debug.h b/gdb/target-debug.h
>> index 14196b4..068495e 100644
>> --- a/gdb/target-debug.h
>> +++ b/gdb/target-debug.h
>> @@ -116,7 +116,7 @@
>>    target_debug_do_print (host_address_to_string (X))
>>  #define target_debug_print_mem_region_vector(X) \
>>    target_debug_do_print (host_address_to_string (X.data ()))
>> -#define target_debug_print_VEC_static_tracepoint_marker_p__p(X)	\
>> +#define target_debug_print_VEC_static_tracepoint_marker_p_p(X)	\
>>    target_debug_do_print (host_address_to_string (X))
>>  #define target_debug_print_const_struct_target_desc_p(X)	\
>>    target_debug_do_print (host_address_to_string (X))
>> diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
>> index e0d7a9a..1cbe6f8 100644
>> --- a/gdb/target-delegates.c
>> +++ b/gdb/target-delegates.c
>> @@ -3352,7 +3352,7 @@ debug_static_tracepoint_markers_by_strid (struct
>> target_ops *self, const char *a
>>    fputs_unfiltered (", ", gdb_stdlog);
>>    target_debug_print_const_char_p (arg1);
>>    fputs_unfiltered (") = ", gdb_stdlog);
>> -  target_debug_print_VEC_static_tracepoint_marker_p__p (result);
>> +  target_debug_print_VEC_static_tracepoint_marker_p_p (result);
>>    fputs_unfiltered ("\n", gdb_stdlog);
>>    return result;
>>  }
> 
> LGTM, I'll make a patch to remove the mem_region_vector typedef.
> 
> Simon

Hi Pedro,

Do you mind if I rebase and push this patch?  Additional cleanups I have
in line would benefit from it.

Thanks,

Simon
Pedro Alves Dec. 3, 2017, 11:58 a.m. UTC | #3 
> Do you mind if I rebase and push this patch?  Additional cleanups I have
> in line would benefit from it.

Sure, please go ahead.  Thanks.
Simon Marchi Dec. 3, 2017, 5:59 p.m. UTC | #4 
On 2017-12-03 06:58 AM, Pedro Alves wrote:
>> Do you mind if I rebase and push this patch?  Additional cleanups I have
>> in line would benefit from it.
> 
> Sure, please go ahead.  Thanks.

I rebased, build-tested and pushed.

Thanks,

Simon
diff mbox

Patch

diff --git a/gdb/make-target-delegates b/gdb/make-target-delegates
index fd51c64..1773232 100755
--- a/gdb/make-target-delegates
+++ b/gdb/make-target-delegates
@@ -232,8 +232,18 @@  sub munge_type($) {
 	$result = $1;
     } else {
 	($result = $typename) =~ s/\s+$//;
-	$result =~ s/[ ()]/_/g;
+	$result =~ s/[ ()<>:]/_/g;
 	$result =~ s/[*]/p/g;
+
+	# Identifers with double underscores are reserved to the C++
+	# implementation.
+	$result =~ s/_+/_/g;
+
+	# Avoid ending the function name with underscore, for
+	# cosmetics.  Trailing underscores appear after munging types
+	# with template parameters, like e.g. "foo<int>".
+	$result =~ s/_$//g;
+
 	$result = 'target_debug_print_' . $result;
     }
 
diff --git a/gdb/target-debug.h b/gdb/target-debug.h
index 14196b4..068495e 100644
--- a/gdb/target-debug.h
+++ b/gdb/target-debug.h
@@ -116,7 +116,7 @@ 
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_mem_region_vector(X) \
   target_debug_do_print (host_address_to_string (X.data ()))
-#define target_debug_print_VEC_static_tracepoint_marker_p__p(X)	\
+#define target_debug_print_VEC_static_tracepoint_marker_p_p(X)	\
   target_debug_do_print (host_address_to_string (X))
 #define target_debug_print_const_struct_target_desc_p(X)	\
   target_debug_do_print (host_address_to_string (X))
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index e0d7a9a..1cbe6f8 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -3352,7 +3352,7 @@  debug_static_tracepoint_markers_by_strid (struct target_ops *self, const char *a
   fputs_unfiltered (", ", gdb_stdlog);
   target_debug_print_const_char_p (arg1);
   fputs_unfiltered (") = ", gdb_stdlog);
-  target_debug_print_VEC_static_tracepoint_marker_p__p (result);
+  target_debug_print_VEC_static_tracepoint_marker_p_p (result);
   fputs_unfiltered ("\n", gdb_stdlog);
   return result;
 }