[PATCH 05/25] Use visitor pattern for "maint print c-tdesc"
Simon Marchi
simon.marchi@polymtl.ca
Tue Jun 20 23:37:00 GMT 2017
On 2017-06-12 10:41, Yao Qi wrote:
> diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
> index e2dcd1d..ac19792 100644
> --- a/gdb/target-descriptions.c
> +++ b/gdb/target-descriptions.c
> @@ -35,6 +35,23 @@
> #include "hashtab.h"
> #include "inferior.h"
>
> +class tdesc_element_visitor
> +{
> +public:
> + virtual void visit (const target_desc *e) = 0;
> + virtual void visit_end (const target_desc *e) = 0;
For elements which children, and therefore can be visited before and
after their children, I'd suggest using "visit_pre" and "visit_post" for
the methods names. It relates better to the terms pre-order and
post-order.
> @@ -259,6 +287,30 @@ public:
>
> /* The types associated with this feature. */
> VEC(tdesc_type_p) *types = NULL;
> +
> + void accept (tdesc_element_visitor &v) const override
> + {
> + v.visit (this);
> +
> + struct tdesc_type *type;
> +
> + for (int ix = 0;
> + VEC_iterate (tdesc_type_p, types, ix, type);
> + ix++)
> + {
> + type->accept (v);
> + }
No need for braces here (and for all the ->accept calls in for loops).
> +
> + struct tdesc_reg *reg;
> +
> + for (int ix = 0;
> + VEC_iterate (tdesc_reg_p, registers, ix, reg);
> + ix++)
> + {
> + reg->accept (v);
> + }
> + }
> +
> } *tdesc_feature_p;
> DEF_VEC_P(tdesc_feature_p);
>
> @@ -268,23 +320,67 @@ DEF_VEC_P(arch_p);
>
> /* A target description. */
>
> -struct target_desc
> +struct target_desc : tdesc_element
> {
It's not a big deal, but the C++ification of target_desc would have
fitted better in patch 03/25 I guess.
> +public:
You can remove public:.
> @@ -1707,279 +1783,288 @@ unset_tdesc_filename_cmd (char *args, int
> from_tty)
> target_find_description ();
> }
>
> -static void
> -maint_print_c_tdesc_cmd (char *args, int from_tty)
> +class print_c_tdesc : public tdesc_element_visitor
IWBN to have "visitor" in the class name: print_c_tdesc_visitor.
> {
> - const struct target_desc *tdesc;
> - const struct bfd_arch_info *compatible;
> - const char *filename, *inp;
> - char *function, *outp;
> - struct property *prop;
> - struct tdesc_feature *feature;
> - struct tdesc_reg *reg;
> - struct tdesc_type *type;
> - struct tdesc_type_field *f;
> - int ix, ix2, ix3;
> - int printed_field_type = 0;
> +public:
> + print_c_tdesc (std::string &filename_after_features)
> + : m_filename_after_features (filename_after_features)
Could you document the parameter filename_after_features?
I didn't read the whole visitor code (it's getting late here...), but I
like the idea and the pattern.
Simon
More information about the Gdb-patches
mailing list