Re: [PATCH v3 1/2] Reorganize code to handle TYPE_CODE_{STRUCT,UNION} on 'c_type_print_base'

[Simon Marchi <simon.marchi@ericsson.com>]

Hi Sergio,

LGTM, with some nits.

On 2017-12-11 02:57 PM, Sergio Durigan Junior wrote:
> diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
> index f3c3e7d706..46c814fae8 100644
> --- a/gdb/c-typeprint.c
> +++ b/gdb/c-typeprint.c
> @@ -875,6 +875,458 @@ output_access_specifier (struct ui_file *stream,
>    return last_access;
>  }
>  
> +/* Return true is an access label (i.e., "public:", "private:",
> +   "protected:") needs to be printed for TYPE.  */
> +
> +static bool
> +need_access_label_p (struct type *type)
> +{
> +  bool need_access_label = false;
> +  int i, j;
> +  int len, len2;
> +
> +  if (TYPE_DECLARED_CLASS (type))
> +    {
> +      QUIT;
> +      len = TYPE_NFIELDS (type);
> +      for (i = TYPE_N_BASECLASSES (type); i < len; i++)
> +	if (!TYPE_FIELD_PRIVATE (type, i))
> +	  {
> +	    need_access_label = true;
> +	    break;
> +	  }
> +      QUIT;
> +      if (!need_access_label)
> +	{
> +	  len2 = TYPE_NFN_FIELDS (type);
> +	  for (j = 0; j < len2; j++)
> +	    {
> +	      len = TYPE_FN_FIELDLIST_LENGTH (type, j);
> +	      for (i = 0; i < len; i++)
> +		if (!TYPE_FN_FIELD_PRIVATE (TYPE_FN_FIELDLIST1 (type,
> +								j), i))
> +		  {
> +		    need_access_label = true;
> +		    break;
> +		  }
> +	      if (need_access_label)
> +		break;
> +	    }
> +	}
> +      QUIT;
> +      if (!need_access_label)
> +	{
> +	  for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i)
> +	    {
> +	      if (!TYPE_TYPEDEF_FIELD_PRIVATE (type, i))
> +		{
> +		  need_access_label = true;
> +		  break;
> +		}
> +	    }
> +	}
> +    }
> +  else
> +    {
> +      QUIT;
> +      len = TYPE_NFIELDS (type);
> +      for (i = TYPE_N_BASECLASSES (type); i < len; i++)
> +	if (TYPE_FIELD_PRIVATE (type, i)
> +	    || TYPE_FIELD_PROTECTED (type, i))
> +	  {
> +	    need_access_label = true;
> +	    break;
> +	  }
> +      QUIT;
> +      if (!need_access_label)
> +	{
> +	  len2 = TYPE_NFN_FIELDS (type);
> +	  for (j = 0; j < len2; j++)
> +	    {
> +	      QUIT;
> +	      len = TYPE_FN_FIELDLIST_LENGTH (type, j);
> +	      for (i = 0; i < len; i++)
> +		if (TYPE_FN_FIELD_PROTECTED (TYPE_FN_FIELDLIST1 (type,
> +								 j), i)
> +		    || TYPE_FN_FIELD_PRIVATE (TYPE_FN_FIELDLIST1 (type,
> +								  j),
> +					      i))
> +		  {
> +		    need_access_label = true;
> +		    break;
> +		  }
> +	      if (need_access_label)
> +		break;
> +	    }
> +	}
> +      QUIT;
> +      if (!need_access_label)
> +	{
> +	  for (i = 0; i < TYPE_TYPEDEF_FIELD_COUNT (type); ++i)
> +	    {
> +	      if (TYPE_TYPEDEF_FIELD_PROTECTED (type, i)
> +		  || TYPE_TYPEDEF_FIELD_PRIVATE (type, i))
> +		{
> +		  need_access_label = true;
> +		  break;
> +		}
> +	    }
> +	}
> +    }
> +
> +  return need_access_label;

It seems to me that you could return immediately when you detect that a label is
needed.  It would allow removing a bunch of if (!need_access_label).

> @@ -898,10 +1350,8 @@ c_type_print_base (struct type *type, struct ui_file *stream,
>  		   int show, int level, const struct type_print_options *flags)
>  {
>    int i;
> -  int len, real_len;
> -  enum access_specifier section_type;
> -  int need_access_label = 0;
> -  int j, len2;
> +  int len;
> +  int j;

j is unused.

Thanks,

Simon