From de7d3331ec7184b6fe89461be8cadf25326dbf62 Mon Sep 17 00:00:00 2001 From: Dodji Seketeli Date: Mon, 27 Jul 2020 16:55:09 +0200 Subject: [PATCH 1/2] Pimpl-ify traversable_base and remove its unused traverse method The traverse method is defined in the traversable_base class but is never used as the synonymous methods in the derived classes have different argument types and so hide it. It's never used because it's mostly intended as documentation for what the implementations of this interface should look like. Namely they should define a traversable method and its parameter type should derive from the parameter type of traversable_base::traverse. But apparently, clang's -Werror-overloaded-virtual is not happy about this. It flags it as an error. It's not. But hey, let's work-around it then. So this patch just comments that method out and document its intent. To make the change somewhat useful, this patch pimpl-ifies this abg-traverse.h header file to get us one step closer to some a{b,p}i stability. The definitions are moved into abg-traverse.cc. * include/abg-traverse.h (traversable_base::priv): Declare new type. (traverse_base::priv_sptr): Add pointer to private data member. (traverse_base::visiting_): Move this data member definition into traverse_base::priv. (traverse_base::{visiting, traverse_base, ~traverse_base}): Move definitions out-of-oline. (traverse_base::traverse): Comment out. * src/abg-traverse.cc (struct traversable_base::priv): Define new type. (traversable_base::{traversable_base, ~traversable_base, traverse, visiting}): Move these previous inline definitions here. Signed-off-by: Dodji Seketeli --- include/abg-traverse.h | 52 ++++++++++++++++++++++++-------------------------- src/abg-traverse.cc | 41 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/include/abg-traverse.h b/include/abg-traverse.h index a7170da..92a8a24 100644 --- a/include/abg-traverse.h +++ b/include/abg-traverse.h @@ -44,40 +44,30 @@ struct node_visitor_base /// The interface for types which are feeling social and want to be /// visited during the traversal of a hierarchy of nodes. +/// +/// It is expected that classes derived from traversable_base define a +/// traverse method specialised to the node *visitor* type. Such +/// methods should visit nodes recursively, calling +/// ir_node_visitor::visit for each node. The method should return +/// true until all nodes have been visited. class traversable_base { - bool visiting_; + struct priv; + typedef shared_ptr priv_sptr; + + priv_sptr priv_; protected: - /// This should returns false before and after the node has been - /// visiting. During the visiting of the node (and of its children) - /// this should return true. - /// - /// @return true if the current node is being visited. - bool - visiting() - {return visiting_;} + traversable_base(); - /// The traversing code should be responsible of calling this, not - /// the user code. - /// - /// This is the setter of the "visiting" flag of the node being - /// visited. If set to yes, it means the node is being visited. - /// False means either the node has not yet been visited, or it - /// has already been visited. - /// - /// @param f the new value of the "visiting" flag. - void - visiting(bool f) - {visiting_ = f;} + bool visiting() const; + + void visiting(bool f); public: - traversable_base() - : visiting_() - {} - virtual ~traversable_base() {} + virtual ~traversable_base(); /// This virtual method is overloaded and implemented by any single /// type which instance is going to be visited during the traversal @@ -92,8 +82,16 @@ public: /// /// @return true if traversed until the end of the type tree, false /// otherwise. - virtual bool traverse(node_visitor_base& v); -}; + /// + /// Note that each class that derives from this one and overloads + /// this method will have to define a type for its node visitor + /// argument (the one named v). That type will have to derive from + /// the node_visitor_base type. In that sense, this new overload + /// method will "hide" this one. That is why we are keeping this + /// method commented, for documentation purposes. + + // virtual bool traverse(node_visitor_base& v); +}; // end class traversable_base }// end namespace ir. }//end namespace abigail diff --git a/src/abg-traverse.cc b/src/abg-traverse.cc index 3fb87ea..a3cb797 100644 --- a/src/abg-traverse.cc +++ b/src/abg-traverse.cc @@ -35,9 +35,46 @@ namespace abigail namespace ir { +/// Private data type of the @ref traversable_base type. +struct traversable_base::priv +{ + bool visiting_; + + priv(bool visiting = false) + : visiting_(visiting) + {} +}; // end struct traversable_base::priv + +/// Default constructor of the @ref traversable_base type. +traversable_base::traversable_base() + :priv_(new priv) +{} + +/// Destructor of the @ref traversable_base type. +traversable_base::~traversable_base() +{} + +/// This should returns false before and after the node has been +/// visiting. During the visiting of the node (and of its children) +/// this should return true. +/// +/// @return true if the current node is being visited. bool -traversable_base::traverse(node_visitor_base&) -{return true;} +traversable_base::visiting() const +{return priv_->visiting_;} + +/// The traversing code should be responsible of calling this, not +/// the user code. +/// +/// This is the setter of the "visiting" flag of the node being +/// visited. If set to yes, it means the node is being visited. +/// False means either the node has not yet been visited, or it +/// has already been visited. +/// +/// @param f the new value of the "visiting" flag. +void +traversable_base::visiting(bool f) +{priv_->visiting_ = f;} }// end namaspace ir }// end namespace abigail -- 1.8.3.1