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

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Monday, December 11 2017, Simon Marchi wrote:

> Hi Sergio,
>
> LGTM, with some nits.

Thanks for the review.

> 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).

Done.

>> @@ -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.

Removed.

Thanks,

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/