This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFAv2 3/6] Add [-q] [-t TYPEREGEXP] [NAMEREGEXP] args to info [args|functions|locals|variables]
- From: Philippe Waroquiers <philippe dot waroquiers at skynet dot be>
- To: Tom Tromey <tom at tromey dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Sun, 23 Sep 2018 23:34:09 +0200
- Subject: Re: [RFAv2 3/6] Add [-q] [-t TYPEREGEXP] [NAMEREGEXP] args to info [args|functions|locals|variables]
- References: <20180826165359.1600-1-philippe.waroquiers@skynet.be> <20180826165359.1600-4-philippe.waroquiers@skynet.be> <874lemu7kz.fsf@tromey.com>
Feedback below ...
RFAv3 will follow soon
Thanks for the comments.
Philippe
On Tue, 2018-09-18 at 09:51 -0600, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
>
> Philippe> As stack.h now uses a type defined in gdb_regex.h, all files including stack.h
> Philippe> need to include gdb_regex.h.
>
> It's better to just include gdb_regex.h from stack.h in this situation.
Done (rather, no need anymore for regexp in stack.h).
>
> Philippe> + if (preg
>
> I think preg.has_value (). gdb is trending away from these bool
> conversions, I think. At least that's been the style for pointers.
>
> Philippe> + if (treg
>
> Here too.
Done.
>
> Philippe> +/* Prepares the regular expression REG from REGEXP.
> Philippe> + If REGEXP is NULL, it results in an empty regular expression. */
> Philippe> +static void
> Philippe> +prepare_reg (const char *regexp, gdb::optional<compiled_regex> ®)
>
> Out parameters should be pointers in the gdb style.
Done.
>
> Philippe> - iterate_over_block_local_vars (block,
> Philippe> + iterate_over_block_local_vars (block, preg, treg,
> Philippe> do_print_variable_and_value,
> Philippe> &cb_data);
>
> iterate_over_block_local_vars already takes some opaque data that it
> just passes through to the callback. So, I think there should not be
> a need to add the regexp parameters directly to it. Instead they can be
> put into the cb_data here.
>
> Philippe> void
> Philippe> iterate_over_block_arg_vars (const struct block *b,
> Philippe> + const gdb::optional<compiled_regex> &preg,
> Philippe> + const gdb::optional<compiled_regex> &treg,
> Philippe> iterate_over_block_arg_local_vars_cb cb,
> Philippe> void *cb_data)
>
> Same here.
Done: the preg/treg regexp have been put in the opaque cb_data,
and regexp filtering logic has been moved to do_print_variable_and_value.
>
> Philippe> +/* Returns true if the type_name of symbol_type of SYM matches TREG.
> Philippe> + If SYM has no symbol_type or symbol_name, returns false. */
> Philippe> +
> Philippe> +bool
> Philippe> +treg_matches_sym_type_name (const compiled_regex &treg,
> Philippe> + const struct symbol *sym)
> Philippe> +{
> Philippe> + if (language_mode == language_mode_auto)
> Philippe> + language_def (SYMBOL_LANGUAGE (sym))->la_print_type
> Philippe> + (sym_type, "", &printed_sym_type_name,
> Philippe> + -1, 0, &default_ptype_flags);
>
> I think this won't actually work, because the type printing code is free
> to use current_language. This is not good, but it is how it is.
>
> So instead I think what this should do is save and restore the current
> language. IIRC there's a helper RAII class for that already.
>
> Then you can just call type_print and drop patch #2.
Done.
>
> Philippe> +extern bool
> Philippe> +treg_matches_sym_type_name (const compiled_regex &treg,
> Philippe> + const struct symbol *sym);
>
> No newline after the "bool" for a declaration.
Done.
Thanks
Philippe