[librsvg: 7/8] Turn RsvgNode into a newtype on Rc<Node<NodeData>>

[Federico Mena Quintero <federico src gnome org>]

commit 10321f2a223267c4adcfdb1d26e0977bb469b192
Author: Federico Mena Quintero <federico gnome org>
Date:   Fri May 10 18:58:07 2019 -0500

    Turn RsvgNode into a newtype on Rc<Node<NodeData>>
    
    Previously this was a type alias,
    
        pub type RsvgNode = Rc<Node>;
    
    But now we have a newtype
    
        pub type RsvgNode = NodeRef<NodeData>;   // in node.rs
    
        pub struct NodeRef<T>(pub Rc<Node<T>>);  // in tree_utils/mod.rs
    
    The newtype similar to "NodeRef" in the Kuchiki crate.  Later we will
    implement the selectors::Element trait on that RsvgNode newtype;
    whereas beforehand, we wouldn't be able to implement a trait on a
    plain Rc<> because of the orphan rule.

 rsvg_internals/src/create_node.rs    |  2 +-
 rsvg_internals/src/drawing_ctx.rs    |  6 +--
 rsvg_internals/src/filters/mod.rs    | 34 +++++++++------
 rsvg_internals/src/handle.rs         |  2 +-
 rsvg_internals/src/node.rs           | 82 +++++++++++++++++++++---------------
 rsvg_internals/src/pattern.rs        |  7 ++-
 rsvg_internals/src/structure.rs      |  1 +
 rsvg_internals/src/tree_utils/mod.rs | 68 ++++++++++++++++++++++--------
 rsvg_internals/src/xml.rs            | 15 +++----
 9 files changed, 136 insertions(+), 81 deletions(-)
---
diff --git a/rsvg_internals/src/create_node.rs b/rsvg_internals/src/create_node.rs
index cfcb1e0d..10a1724a 100644
--- a/rsvg_internals/src/create_node.rs
+++ b/rsvg_internals/src/create_node.rs
@@ -37,7 +37,7 @@ use crate::text::{NodeTRef, NodeTSpan, NodeText};
 macro_rules! node_create_fn {
     ($name:ident, $node_type:ident, $new_fn:expr) => {
         fn $name(id: Option<&str>, class: Option<&str>, parent: Option<&RsvgNode>) -> RsvgNode {
-            node_new(NodeType::$node_type, parent, id, class, Box::new($new_fn()))
+            RsvgNode::new(NodeType::$node_type, parent, id, class, Box::new($new_fn()))
         }
     };
 }
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index 41dbc274..35f53fd9 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -824,7 +824,7 @@ impl DrawingCtx {
         let stack_top = self.drawsub_stack.pop();
 
         let draw = if let Some(ref top) = stack_top {
-            Rc::ptr_eq(top, node)
+            top == node
         } else {
             true
         };
@@ -1067,7 +1067,7 @@ impl Drop for AcquiredNode {
     fn drop(&mut self) {
         let mut stack = self.0.borrow_mut();
         let last = stack.pop().unwrap();
-        assert!(Rc::ptr_eq(&last, &self.1));
+        assert!(last == self.1);
     }
 }
 
@@ -1163,6 +1163,6 @@ impl NodeStack {
     }
 
     pub fn contains(&self, node: &RsvgNode) -> bool {
-        self.0.iter().find(|n| Rc::ptr_eq(n, node)).is_some()
+        self.0.iter().find(|n| **n == *node).is_some()
     }
 }
diff --git a/rsvg_internals/src/filters/mod.rs b/rsvg_internals/src/filters/mod.rs
index 8613c599..ce903ec2 100644
--- a/rsvg_internals/src/filters/mod.rs
+++ b/rsvg_internals/src/filters/mod.rs
@@ -11,12 +11,13 @@ use crate::coord_units::CoordUnits;
 use crate::drawing_ctx::DrawingCtx;
 use crate::error::{RenderingError, ValueErrorKind};
 use crate::length::{LengthHorizontal, LengthUnit, LengthVertical};
-use crate::node::{NodeResult, NodeTrait, NodeType, RsvgNode};
+use crate::node::{NodeData, NodeResult, NodeTrait, NodeType, RsvgNode};
 use crate::parsers::{ParseError, ParseValue};
-use crate::property_defs::ColorInterpolationFilters;
-use crate::property_bag::PropertyBag;
 use crate::properties::ComputedValues;
+use crate::property_bag::PropertyBag;
+use crate::property_defs::ColorInterpolationFilters;
 use crate::surface_utils::shared_surface::{SharedImageSurface, SurfaceType};
+use crate::tree_utils::{Node, NodeRef};
 
 mod bounds;
 use self::bounds::BoundsBuilder;
@@ -303,24 +304,27 @@ pub fn render(
 
             (c, linear_rgb)
         })
+        // Keep only filter primitives (those that implement the Filter trait)
         .filter_map(|(c, linear_rgb)| {
-            let rr = RcRef::new(c)
-                .try_map(|c| {
+            let rr = RcRef::new(c.0)
+                .try_map(|c: &Node<NodeData>| {
                     // Go through the filter primitives and see if the node is one of them.
                     #[inline]
                     fn as_filter<T: Filter>(x: &T) -> &Filter {
                         x
                     }
 
-                    // Unfortunately it's not possible to downcast to a trait object. If we could
-                    // attach arbitrary data to nodes it would really help here. Previously that
-                    // arbitrary data was in form of RsvgCNodeImpl, but that was heavily tuned for
-                    // storing C pointers with all subsequent downsides.
+                    // Unfortunately it's not possible to downcast to a trait object.  So, we
+                    // try downcasting to each filter type individually, and use
+                    // owning_ref::RcRef to reduce the children to a list of
+                    // OwningRef<Rc<Node<NodeData>>, dyn Filter> -- which is essentially the
+                    // NodeRef, and something inside the NodeRef that implements the Filter
+                    // trait.
                     macro_rules! try_downcasting_to_filter {
                         ($c:expr; $($t:ty),+$(,)*) => ({
                             let mut filter = None;
                             $(
-                                filter = filter.or_else(|| $c.get_impl::<$t>().map(as_filter));
+                                filter = filter.or_else(|| $c.borrow().get_impl::<$t>().map(as_filter));
                             )+
                             filter
                         })
@@ -351,14 +355,18 @@ pub fn render(
         });
 
     for (rr, linear_rgb) in primitives {
+        // rr: OwningRef<Rc<Node<NodeData>>, dyn Filter>
+
+        let rr_node = NodeRef(rr.as_owner().clone());
+
         let mut render = |filter_ctx: &mut FilterContext| {
             if let Err(err) = rr
-                .render(rr.as_owner(), filter_ctx, draw_ctx)
+                .render(&rr_node, filter_ctx, draw_ctx)
                 .and_then(|result| filter_ctx.store_result(result))
             {
                 rsvg_log!(
                     "(filter primitive {} returned an error: {})",
-                    rr.as_owner().get_human_readable_name(),
+                    rr_node.get_human_readable_name(),
                     err
                 );
 
@@ -382,7 +390,7 @@ pub fn render(
         let elapsed = start.elapsed();
         rsvg_log!(
             "(rendered filter primitive {} in\n    {} seconds)",
-            rr.as_owner().get_human_readable_name(),
+            rr_node.get_human_readable_name(),
             elapsed.as_secs() as f64 + f64::from(elapsed.subsec_nanos()) / 1e9
         );
     }
diff --git a/rsvg_internals/src/handle.rs b/rsvg_internals/src/handle.rs
index c32c2a9a..36b8dc01 100644
--- a/rsvg_internals/src/handle.rs
+++ b/rsvg_internals/src/handle.rs
@@ -233,7 +233,7 @@ impl Handle {
         let node = self.get_node_or_root(id)?;
 
         let root = self.svg.root();
-        let is_root = Rc::ptr_eq(&node, &root);
+        let is_root = node == root;
 
         if is_root {
             let cascaded = node.get_cascaded_values();
diff --git a/rsvg_internals/src/node.rs b/rsvg_internals/src/node.rs
index 38f666e8..04e98e60 100644
--- a/rsvg_internals/src/node.rs
+++ b/rsvg_internals/src/node.rs
@@ -13,12 +13,14 @@ use crate::parsers::Parse;
 use crate::properties::{ComputedValues, SpecifiedValue, SpecifiedValues};
 use crate::property_bag::PropertyBag;
 use crate::property_defs::Overflow;
-use crate::tree_utils;
+use crate::tree_utils::{self, NodeRef, NodeWeakRef};
 use locale_config::Locale;
 
-/// A strong reference to a node
-pub type RsvgNode = Rc<Node>;
+/// Tree node with specific data
+pub type RsvgNode = NodeRef<NodeData>;
+pub type RsvgWeakNode = NodeWeakRef<NodeData>;
 
+/// Contents of a tree node
 pub struct NodeData {
     node_type: NodeType,
     id: Option<String>,    // id attribute from XML element
@@ -33,7 +35,45 @@ pub struct NodeData {
     style_attr: RefCell<String>,
 }
 
-pub type Node = tree_utils::Node<NodeData>;
+impl NodeRef<NodeData> {
+    pub fn new(
+        node_type: NodeType,
+        parent: Option<&NodeRef<NodeData>>,
+        id: Option<&str>,
+        class: Option<&str>,
+        node_impl: Box<NodeTrait>,
+    ) -> NodeRef<NodeData> {
+        let data = NodeData {
+            node_type,
+            id: id.map(str::to_string),
+            class: class.map(str::to_string),
+            specified_values: RefCell::new(Default::default()),
+            important_styles: Default::default(),
+            transform: Cell::new(Matrix::identity()),
+            result: RefCell::new(Ok(())),
+            values: RefCell::new(ComputedValues::default()),
+            cond: Cell::new(true),
+            node_impl,
+            style_attr: RefCell::new(String::new()),
+        };
+
+        NodeRef(Rc::new(tree_utils::Node::new(data, parent)))
+    }
+
+    pub fn downgrade(&self) -> RsvgWeakNode {
+        Rc::downgrade(&self.0)
+    }
+
+    pub fn upgrade(weak: &RsvgWeakNode) -> Option<NodeRef<NodeData>> {
+        weak.upgrade().map(NodeRef)
+    }
+}
+
+impl NodeData {
+    pub fn get_impl<T: NodeTrait>(&self) -> Option<&T> {
+        (&self.node_impl).downcast_ref::<T>()
+    }
+}
 
 /// Can obtain computed values from a node
 ///
@@ -60,7 +100,7 @@ impl<'a> CascadedValues<'a> {
     /// This is what nodes should normally use to draw their children from their `draw()` method.
     /// Nodes that need to override the cascade for their children can use `new_from_values()`
     /// instead.
-    pub fn new(&self, node: &'a Node) -> CascadedValues<'a> {
+    pub fn new(&self, node: &'a RsvgNode) -> CascadedValues<'a> {
         match self.inner {
             CascadedInner::FromNode(_) => CascadedValues {
                 inner: CascadedInner::FromNode(node.borrow().values.borrow()),
@@ -75,7 +115,7 @@ impl<'a> CascadedValues<'a> {
     /// This is to be used only in the toplevel drawing function, or in elements like `<marker>`
     /// that don't propagate their parent's cascade to their children.  All others should use
     /// `new()` to derive the cascade from an existing one.
-    fn new_from_node(node: &Node) -> CascadedValues<'_> {
+    fn new_from_node(node: &RsvgNode) -> CascadedValues<'_> {
         CascadedValues {
             inner: CascadedInner::FromNode(node.borrow().values.borrow()),
         }
@@ -86,7 +126,7 @@ impl<'a> CascadedValues<'a> {
     ///
     /// This is for the `<use>` element, which draws the element which it references with the
     /// `<use>`'s own cascade, not wih the element's original cascade.
-    pub fn new_from_values(node: &'a Node, values: &ComputedValues) -> CascadedValues<'a> {
+    pub fn new_from_values(node: &'a RsvgNode, values: &ComputedValues) -> CascadedValues<'a> {
         let mut v = values.clone();
         node.borrow()
             .specified_values
@@ -274,7 +314,7 @@ impl NodeType {
     }
 }
 
-impl Node {
+impl RsvgNode {
     pub fn get_type(&self) -> NodeType {
         self.borrow().node_type
     }
@@ -537,7 +577,7 @@ impl Node {
     }
 
     pub fn get_impl<T: NodeTrait>(&self) -> Option<&T> {
-        (&self.borrow().node_impl).downcast_ref::<T>()
+        self.borrow().get_impl()
     }
 
     pub fn draw_children(
@@ -566,27 +606,3 @@ impl Node {
         specified_values.overflow = SpecifiedValue::Specified(Overflow::Hidden);
     }
 }
-
-pub fn node_new(
-    node_type: NodeType,
-    parent: Option<&RsvgNode>,
-    id: Option<&str>,
-    class: Option<&str>,
-    node_impl: Box<NodeTrait>,
-) -> RsvgNode {
-    let data = NodeData {
-        node_type,
-        id: id.map(str::to_string),
-        class: class.map(str::to_string),
-        specified_values: RefCell::new(Default::default()),
-        important_styles: Default::default(),
-        transform: Cell::new(Matrix::identity()),
-        result: RefCell::new(Ok(())),
-        values: RefCell::new(ComputedValues::default()),
-        cond: Cell::new(true),
-        node_impl,
-        style_attr: RefCell::new(String::new()),
-    };
-
-    Rc::new(tree_utils::Node::<NodeData>::new(data, parent))
-}
diff --git a/rsvg_internals/src/pattern.rs b/rsvg_internals/src/pattern.rs
index 90077096..54dc9934 100644
--- a/rsvg_internals/src/pattern.rs
+++ b/rsvg_internals/src/pattern.rs
@@ -2,7 +2,6 @@ use cairo;
 use cairo::{MatrixTrait, PatternTrait};
 use std::cell::RefCell;
 use std::f64;
-use std::rc::*;
 
 use crate::allowed_url::Fragment;
 use crate::aspect_ratio::*;
@@ -45,7 +44,7 @@ pub struct Pattern {
     // Point back to our corresponding node, or to the fallback node which has children.
     // If the value is None, it means we are fully resolved and didn't find any children
     // among the fallbacks.
-    pub node: Option<Weak<Node>>,
+    pub node: Option<RsvgWeakNode>,
 }
 
 impl Default for Pattern {
@@ -179,7 +178,7 @@ impl NodeTrait for NodePattern {
 
         let mut p = self.pattern.borrow_mut();
 
-        p.node = Some(Rc::downgrade(node));
+        p.node = Some(node.downgrade());
 
         for (attr, value) in pbag.iter() {
             match attr {
@@ -422,7 +421,7 @@ impl PaintSource for NodePattern {
         // Set up transformations to be determined by the contents units
 
         // Draw everything
-        let pattern_node = pattern.node.clone().unwrap().upgrade().unwrap();
+        let pattern_node = RsvgNode::upgrade(pattern.node.as_ref().unwrap()).unwrap();
         let pattern_cascaded = pattern_node.get_cascaded_values();
         let pattern_values = pattern_cascaded.get();
 
diff --git a/rsvg_internals/src/structure.rs b/rsvg_internals/src/structure.rs
index ce45d124..76998774 100644
--- a/rsvg_internals/src/structure.rs
+++ b/rsvg_internals/src/structure.rs
@@ -17,6 +17,7 @@ use crate::properties::ComputedValues;
 use crate::property_bag::PropertyBag;
 use crate::property_defs::Overflow;
 use crate::rect::RectangleExt;
+use crate::tree_utils::Node;
 use crate::viewbox::*;
 
 pub struct NodeGroup();
diff --git a/rsvg_internals/src/tree_utils/mod.rs b/rsvg_internals/src/tree_utils/mod.rs
index 66fde4ea..885d11dc 100644
--- a/rsvg_internals/src/tree_utils/mod.rs
+++ b/rsvg_internals/src/tree_utils/mod.rs
@@ -1,9 +1,39 @@
 use std::cell::RefCell;
+use std::ops::Deref;
 use std::rc::{Rc, Weak};
 
-pub type NodeRef<T> = Rc<Node<T>>;
+/// A strong reference to a node
+///
+/// These strong references can be compared with `==` as they
+/// implement `PartialEq` and `Eq`; comparison means whether they
+/// point to the same node struct.
+pub struct NodeRef<T>(pub Rc<Node<T>>);
+
 pub type NodeWeakRef<T> = Weak<Node<T>>;
 
+impl<T> Clone for NodeRef<T> {
+    fn clone(&self) -> NodeRef<T> {
+        NodeRef(self.0.clone())
+    }
+}
+
+impl<T> Deref for NodeRef<T> {
+    type Target = Node<T>;
+
+    #[inline]
+    fn deref(&self) -> &Node<T> {
+        &*self.0
+    }
+}
+
+impl<T> Eq for NodeRef<T> {}
+impl<T> PartialEq for NodeRef<T> {
+    #[inline]
+    fn eq(&self, other: &NodeRef<T>) -> bool {
+        Rc::ptr_eq(&self.0, &other.0)
+    }
+}
+
 pub struct Node<T> {
     parent: Option<NodeWeakRef<T>>,
     first_child: RefCell<Option<NodeRef<T>>>,
@@ -16,7 +46,7 @@ pub struct Node<T> {
 impl<T> Node<T> {
     pub fn new(data: T, parent: Option<&NodeRef<T>>) -> Node<T> {
         Node {
-            parent: parent.map(Rc::downgrade),
+            parent: parent.map(|n| Rc::downgrade(&n.0)),
             first_child: RefCell::new(None),
             last_child: RefCell::new(None),
             next_sibling: RefCell::new(None),
@@ -28,7 +58,7 @@ impl<T> Node<T> {
     pub fn parent(&self) -> Option<NodeRef<T>> {
         match self.parent {
             None => None,
-            Some(ref weak_node) => Some(weak_node.upgrade().unwrap()),
+            Some(ref weak_node) => Some(NodeRef(weak_node.upgrade().unwrap())),
         }
     }
 
@@ -42,7 +72,7 @@ impl<T> Node<T> {
     pub fn last_child(&self) -> Option<NodeRef<T>> {
         match *self.last_child.borrow() {
             None => None,
-            Some(ref weak_node) => Some(weak_node.upgrade().unwrap()),
+            Some(ref weak_node) => Some(NodeRef(weak_node.upgrade().unwrap())),
         }
     }
 
@@ -56,7 +86,7 @@ impl<T> Node<T> {
     pub fn previous_sibling(&self) -> Option<NodeRef<T>> {
         match *self.previous_sibling.borrow() {
             None => None,
-            Some(ref weak_node) => Some(weak_node.upgrade().unwrap()),
+            Some(ref weak_node) => Some(NodeRef(weak_node.upgrade().unwrap())),
         }
     }
 
@@ -64,7 +94,7 @@ impl<T> Node<T> {
         let mut desc = Some(descendant.clone());
 
         while let Some(ref d) = desc.clone() {
-            if Rc::ptr_eq(&ancestor, d) {
+            if ancestor == *d {
                 return true;
             }
 
@@ -78,7 +108,7 @@ impl<T> Node<T> {
         assert!(child.next_sibling.borrow().is_none());
         assert!(child.previous_sibling.borrow().is_none());
 
-        if let Some(last_child_weak) = self.last_child.replace(Some(Rc::downgrade(child))) {
+        if let Some(last_child_weak) = self.last_child.replace(Some(Rc::downgrade(&child.0))) {
             if let Some(last_child) = last_child_weak.upgrade() {
                 child.previous_sibling.replace(Some(last_child_weak));
                 last_child.next_sibling.replace(Some(child.clone()));
@@ -93,7 +123,8 @@ impl<T> Node<T> {
             .last_child
             .borrow()
             .as_ref()
-            .and_then(|child_weak| child_weak.upgrade());
+            .and_then(|child_weak| child_weak.upgrade())
+            .map(NodeRef);
         Children::new(self.first_child.borrow().clone(), last_child)
     }
 
@@ -168,7 +199,7 @@ fn take_if_unique_strong<T>(r: &RefCell<Option<NodeRef<T>>>) -> Option<NodeRef<T
 
     let has_single_ref = match *r {
         None => false,
-        Some(ref rc) if Rc::strong_count(rc) > 1 => false,
+        Some(ref rc) if Rc::strong_count(&rc.0) > 1 => false,
         Some(_) => true,
     };
 
@@ -241,7 +272,8 @@ impl<T> DoubleEndedIterator for Children<T> {
                 .previous_sibling
                 .borrow()
                 .as_ref()
-                .and_then(|sib_weak| sib_weak.upgrade());
+                .and_then(|sib_weak| sib_weak.upgrade())
+                .map(NodeRef);
             Some(next_back)
         })
     }
@@ -256,14 +288,14 @@ mod tests {
 
     #[test]
     fn node_is_its_own_ancestor() {
-        let node = Rc::new(N::new((), None));;
+        let node = NodeRef(Rc::new(N::new((), None)));
         assert!(Node::is_ancestor(node.clone(), node.clone()));
     }
 
     #[test]
     fn node_is_ancestor_of_child() {
-        let node = Rc::new(N::new((), None));;
-        let child = Rc::new(N::new((), Some(&node)));
+        let node = NodeRef(Rc::new(N::new((), None)));
+        let child = NodeRef(Rc::new(N::new((), Some(&node))));
 
         node.append(&child);
 
@@ -273,9 +305,9 @@ mod tests {
 
     #[test]
     fn node_children_iterator() {
-        let node = Rc::new(N::new((), None));;
-        let child = Rc::new(N::new((), Some(&node)));
-        let second_child = Rc::new(N::new((), Some(&node)));
+        let node = NodeRef(Rc::new(N::new((), None)));
+        let child = NodeRef(Rc::new(N::new((), Some(&node))));
+        let second_child = NodeRef(Rc::new(N::new((), Some(&node))));
 
         node.append(&child);
         node.append(&second_child);
@@ -285,12 +317,12 @@ mod tests {
         let c = children.next();
         assert!(c.is_some());
         let c = c.unwrap();
-        assert!(Rc::ptr_eq(&c, &child));
+        assert!(c == child);
 
         let c = children.next_back();
         assert!(c.is_some());
         let c = c.unwrap();
-        assert!(Rc::ptr_eq(&c, &second_child));
+        assert!(c == second_child);
 
         assert!(children.next().is_none());
         assert!(children.next_back().is_none());
diff --git a/rsvg_internals/src/xml.rs b/rsvg_internals/src/xml.rs
index 927745e1..9a459b0a 100644
--- a/rsvg_internals/src/xml.rs
+++ b/rsvg_internals/src/xml.rs
@@ -4,7 +4,6 @@ use encoding::DecoderTrap;
 use glib::IsA;
 use libc;
 use std::collections::HashMap;
-use std::rc::Rc;
 use std::str;
 
 use crate::allowed_url::AllowedUrl;
@@ -14,7 +13,7 @@ use crate::css::CssRules;
 use crate::error::LoadingError;
 use crate::handle::LoadOptions;
 use crate::io::{self, get_input_stream_for_loading};
-use crate::node::{node_new, Node, NodeType, RsvgNode};
+use crate::node::{NodeType, RsvgNode};
 use crate::property_bag::PropertyBag;
 use crate::style::NodeStyle;
 use crate::svg::Svg;
@@ -67,11 +66,11 @@ extern "C" {
 /// trait objects. Normally the context refers to a `NodeCreationContext` implementation which is
 /// what creates normal graphical elements.
 pub struct XmlState {
-    tree_root: Option<Rc<Node>>,
+    tree_root: Option<RsvgNode>,
     ids: Option<HashMap<String, RsvgNode>>,
     css_rules: Option<CssRules>,
     context_stack: Vec<Context>,
-    current_node: Option<Rc<Node>>,
+    current_node: Option<RsvgNode>,
 
     entities: HashMap<String, XmlEntityPtr>,
 
@@ -103,7 +102,7 @@ impl XmlState {
         }
     }
 
-    fn set_root(&mut self, root: &Rc<Node>) {
+    fn set_root(&mut self, root: &RsvgNode) {
         if self.tree_root.is_some() {
             panic!("The tree root has already been set");
         }
@@ -294,7 +293,7 @@ impl XmlState {
             {
                 child
             } else {
-                let child = node_new(
+                let child = RsvgNode::new(
                     NodeType::Chars,
                     Some(node),
                     None,
@@ -313,10 +312,10 @@ impl XmlState {
 
     fn create_node(
         &mut self,
-        parent: Option<&Rc<Node>>,
+        parent: Option<&RsvgNode>,
         name: &str,
         pbag: &PropertyBag,
-    ) -> Rc<Node> {
+    ) -> RsvgNode {
         let ids = self.ids.as_mut().unwrap();
 
         let new_node = create_node_and_register_id(name, parent, pbag, ids);