This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 1/2] Reorganize code to handle TYPE_CODE_{STRUCT,UNION} on 'c_type_print_base'
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Sergio Durigan Junior <sergiodj at redhat dot com>, GDB Patches <gdb-patches at sourceware dot org>
- Cc: Tom Tromey <tom at tromey dot com>, Eli Zaretskii <eliz at gnu dot org>
- Date: Mon, 11 Dec 2017 15:54:55 -0500
- Subject: Re: [PATCH v3 1/2] Reorganize code to handle TYPE_CODE_{STRUCT,UNION} on 'c_type_print_base'
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=simon dot marchi at ericsson dot com;
- References: <20171121160709.23248-1-sergiodj@redhat.com> <20171211195757.18044-1-sergiodj@redhat.com> <20171211195757.18044-2-sergiodj@redhat.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
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