diff mbox series

[v2,2/9] Print field accessibility inline

Message ID 20231027-field-bits-v2-2-cbec64f2136a@adacore.com
State New
Headers
Series Remove char-based bitfield macros |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Testing passed

Commit Message

Tom Tromey Oct. 27, 2023, 5:36 p.m. UTC 
  This changes recursive_dump_type to print field accessibility
information "inline".  This is clearer and preserves the information
when the byte vectors are removed.
---
 gdb/gdbtypes.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Comments

Simon Marchi Nov. 3, 2023, 4:10 a.m. UTC | #1 
On 2023-10-27 13:36, Tom Tromey wrote:
> This changes recursive_dump_type to print field accessibility
> information "inline".  This is clearer and preserves the information
> when the byte vectors are removed.
> ---
>  gdb/gdbtypes.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
> index 1de90eff7ee..203728a61d8 100644
> --- a/gdb/gdbtypes.c
> +++ b/gdb/gdbtypes.c
> @@ -5288,12 +5288,21 @@ recursive_dump_type (struct type *type, int spaces)
>  	gdb_printf ("%*s[%d] bitpos %s bitsize %d type ", spaces + 2, "",
>  		    idx, plongest (type->field (idx).loc_bitpos ()),
>  		    type->field (idx).bitsize ());
> -      gdb_printf ("%s name '%s' (%s)\n",
> +      gdb_printf ("%s name '%s' (%s)",
>  		  host_address_to_string (type->field (idx).type ()),
>  		  type->field (idx).name () != NULL
>  		  ? type->field (idx).name ()
>  		  : "<NULL>",
>  		  host_address_to_string (type->field (idx).name ()));
> +      if (TYPE_FIELD_VIRTUAL (type, idx))
> +	gdb_printf (" virtual");
> +      if (TYPE_FIELD_PRIVATE (type, idx))
> +	gdb_printf (" private");
> +      else if (TYPE_FIELD_PROTECTED (type, idx))
> +	gdb_printf (" protected");
> +      else if (TYPE_FIELD_IGNORE (type, idx))
> +	gdb_printf (" ignored");
> +      gdb_printf ("\n");

I know this is probably personal preference and you write things with
fewer empty lines than I doo, but I find it very difficult to read when
it's so packed, it's hard to discern that this is two ifs, not just one
chain if if/else if/else.  Can you add some new lines to separate them
like this?

      if (TYPE_FIELD_VIRTUAL (type, idx))
	gdb_printf (" virtual");

      if (TYPE_FIELD_PRIVATE (type, idx))
	gdb_printf (" private");
      else if (TYPE_FIELD_PROTECTED (type, idx))
	gdb_printf (" protected");
      else if (TYPE_FIELD_IGNORE (type, idx))
	gdb_printf (" ignored");

      gdb_printf ("\n");

Simon
Tom Tromey Nov. 3, 2023, 3:53 p.m. UTC | #2 
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

Simon> I know this is probably personal preference and you write things with
Simon> fewer empty lines than I doo, but I find it very difficult to read when
Simon> it's so packed, it's hard to discern that this is two ifs, not just one
Simon> chain if if/else if/else.  Can you add some new lines to separate them
Simon> like this?

I made this change.

Tom
diff mbox series

Patch

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 1de90eff7ee..203728a61d8 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5288,12 +5288,21 @@  recursive_dump_type (struct type *type, int spaces)
 	gdb_printf ("%*s[%d] bitpos %s bitsize %d type ", spaces + 2, "",
 		    idx, plongest (type->field (idx).loc_bitpos ()),
 		    type->field (idx).bitsize ());
-      gdb_printf ("%s name '%s' (%s)\n",
+      gdb_printf ("%s name '%s' (%s)",
 		  host_address_to_string (type->field (idx).type ()),
 		  type->field (idx).name () != NULL
 		  ? type->field (idx).name ()
 		  : "<NULL>",
 		  host_address_to_string (type->field (idx).name ()));
+      if (TYPE_FIELD_VIRTUAL (type, idx))
+	gdb_printf (" virtual");
+      if (TYPE_FIELD_PRIVATE (type, idx))
+	gdb_printf (" private");
+      else if (TYPE_FIELD_PROTECTED (type, idx))
+	gdb_printf (" protected");
+      else if (TYPE_FIELD_IGNORE (type, idx))
+	gdb_printf (" ignored");
+      gdb_printf ("\n");
       if (type->field (idx).type () != NULL)
 	{
 	  recursive_dump_type (type->field (idx).type (), spaces + 4);