From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28935 invoked by alias); 20 Jun 2017 23:37:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 28918 invoked by uid 89); 20 Jun 2017 23:37:55 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=fitted, visit X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 20 Jun 2017 23:37:53 +0000 Received: by simark.ca (Postfix, from userid 33) id 3D5061E5A2; Tue, 20 Jun 2017 19:37:51 -0400 (EDT) To: Yao Qi Subject: Re: [PATCH 05/25] Use visitor pattern for "maint print c-tdesc" X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Tue, 20 Jun 2017 23:37:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <1497256916-4958-6-git-send-email-yao.qi@linaro.org> References: <1497256916-4958-1-git-send-email-yao.qi@linaro.org> <1497256916-4958-6-git-send-email-yao.qi@linaro.org> Message-ID: <601672073819187e93e7e81e00a44945@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.5 X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00578.txt.bz2 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