[librsvg: 4/5] Move class/id indexes into Attributes

[Marge Bot <marge-bot src gnome org>]

commit 3af3832ec09afcea6e75956febd693bbc04758fe
Author: Michael Howell <michael notriddle com>
Date:   Thu Jul 21 14:55:08 2022 -0700

    Move class/id indexes into Attributes
    
    Part-of: <https://gitlab.gnome.org/GNOME/librsvg/-/merge_requests/715>

 src/element.rs        | 47 ++++++----------------------------
 src/xml/attributes.rs | 70 ++++++++++++++++++++++++++++++++-------------------
 src/xml/mod.rs        |  2 +-
 3 files changed, 53 insertions(+), 66 deletions(-)
---
diff --git a/src/element.rs b/src/element.rs
index 8b993cb6c..4619cda8e 100644
--- a/src/element.rs
+++ b/src/element.rs
@@ -42,7 +42,7 @@ use crate::shapes::{Circle, Ellipse, Line, Path, Polygon, Polyline, Rect};
 use crate::structure::{ClipPath, Group, Link, Mask, NonRendering, Svg, Switch, Symbol, Use};
 use crate::style::Style;
 use crate::text::{TRef, TSpan, Text};
-use crate::xml::{AttributeIndex, Attributes};
+use crate::xml::Attributes;
 
 // After creating/parsing a Element, it will be in a success or an error state.
 // We represent this with a Result, aliased as a ElementResult.  There is no
@@ -94,8 +94,6 @@ pub trait Draw {
 
 pub struct ElementInner<T: SetAttributes + Draw> {
     element_name: QualName,
-    id_idx: Option<AttributeIndex>,    // id attribute from XML element
-    class_idx: Option<AttributeIndex>, // class attribute from XML element
     attributes: Attributes,
     specified_values: SpecifiedValues,
     important_styles: HashSet<QualName>,
@@ -110,16 +108,12 @@ pub struct ElementInner<T: SetAttributes + Draw> {
 impl<T: SetAttributes + Draw> ElementInner<T> {
     fn new(
         element_name: QualName,
-        id_idx: Option<AttributeIndex>,
-        class_idx: Option<AttributeIndex>,
         attributes: Attributes,
         result: Result<(), ElementError>,
         element_impl: T,
     ) -> ElementInner<T> {
         let mut e = Self {
             element_name,
-            id_idx,
-            class_idx,
             attributes,
             specified_values: Default::default(),
             important_styles: Default::default(),
@@ -153,13 +147,11 @@ impl<T: SetAttributes + Draw> ElementInner<T> {
     }
 
     fn get_id(&self) -> Option<&str> {
-        self.id_idx
-            .and_then(|id_idx| self.attributes.get_by_index(id_idx))
+        self.attributes.get_id()
     }
 
     fn get_class(&self) -> Option<&str> {
-        self.class_idx
-            .and_then(|class_idx| self.attributes.get_by_index(class_idx))
+        self.attributes.get_class()
     }
 
     fn inherit_xml_lang(&mut self, parent: Option<Node>) {
@@ -453,18 +445,7 @@ impl Element {
     ///
     /// This operation does not fail.  Unknown element names simply produce a [`NonRendering`]
     /// element.
-    pub fn new(name: &QualName, attrs: Attributes) -> Element {
-        let mut id_idx = None;
-        let mut class_idx = None;
-
-        for (idx, attr, _) in attrs.iter_indexed() {
-            match attr.expanded() {
-                expanded_name!("", "id") => id_idx = Some(idx),
-                expanded_name!("", "class") => class_idx = Some(idx),
-                _ => (),
-            }
-        }
-
+    pub fn new(name: &QualName, mut attrs: Attributes) -> Element {
         let (create_fn, flags): (ElementCreateFn, ElementCreateFlags) = if name.ns == ns!(svg) {
             match ELEMENT_CREATORS.get(name.local.as_ref()) {
                 // hack in the SVG namespace for supported element names
@@ -480,12 +461,12 @@ impl Element {
         };
 
         if flags == ElementCreateFlags::IgnoreClass {
-            class_idx = None;
+            attrs.clear_class();
         };
 
         //    sizes::print_sizes();
 
-        create_fn(name, attrs, id_idx, class_idx)
+        create_fn(name, attrs)
     }
 
     pub fn element_name(&self) -> &QualName {
@@ -604,20 +585,13 @@ impl fmt::Display for Element {
 
 macro_rules! e {
     ($name:ident, $element_type:ident) => {
-        pub fn $name(
-            element_name: &QualName,
-            attributes: Attributes,
-            id_idx: Option<AttributeIndex>,
-            class_idx: Option<AttributeIndex>,
-        ) -> Element {
+        pub fn $name(element_name: &QualName, attributes: Attributes) -> Element {
             let mut element_impl = <$element_type>::default();
 
             let result = element_impl.set_attributes(&attributes);
 
             let element = Element::$element_type(Box::new(ElementInner::new(
                 element_name.clone(),
-                id_idx,
-                class_idx,
                 attributes,
                 result,
                 element_impl,
@@ -701,12 +675,7 @@ mod creators {
 
 use creators::*;
 
-type ElementCreateFn = fn(
-    element_name: &QualName,
-    attributes: Attributes,
-    id_idx: Option<AttributeIndex>,
-    class_idx: Option<AttributeIndex>,
-) -> Element;
+type ElementCreateFn = fn(element_name: &QualName, attributes: Attributes) -> Element;
 
 #[derive(Copy, Clone, PartialEq)]
 enum ElementCreateFlags {
diff --git a/src/xml/attributes.rs b/src/xml/attributes.rs
index 1ef0e7616..9c8ebe96b 100644
--- a/src/xml/attributes.rs
+++ b/src/xml/attributes.rs
@@ -3,7 +3,9 @@
 use std::slice;
 use std::str;
 
-use markup5ever::{namespace_url, LocalName, Namespace, Prefix, QualName};
+use markup5ever::{
+    expanded_name, local_name, namespace_url, ns, LocalName, Namespace, Prefix, QualName,
+};
 use string_cache::DefaultAtom;
 
 use crate::error::{ImplementationLimit, LoadingError};
@@ -16,20 +18,17 @@ use crate::util::{opt_utf8_cstr, utf8_cstr};
 /// string_cache crate.
 pub type AttributeValue = DefaultAtom;
 
-/// Type used to store an index into the attributes list.
-///
-/// Searching by name requires a linear scan, while looking up by index happens in
-/// constant time.
-#[derive(Clone, Copy)]
-pub struct AttributeIndex(u16);
-
 /// Iterable wrapper for libxml2's representation of attribute/value.
 ///
 /// See the [`new_from_xml2_attributes`] function for information.
 ///
 /// [`new_from_xml2_attributes`]: #method.new_from_xml2_attributes
 #[derive(Clone)]
-pub struct Attributes(Box<[(QualName, AttributeValue)]>);
+pub struct Attributes {
+    attrs: Box<[(QualName, AttributeValue)]>,
+    id_idx: Option<u16>,
+    class_idx: Option<u16>,
+}
 
 /// Iterator from `Attributes.iter`.
 pub struct AttributesIter<'a>(slice::Iter<'a, (QualName, AttributeValue)>);
@@ -37,7 +36,11 @@ pub struct AttributesIter<'a>(slice::Iter<'a, (QualName, AttributeValue)>);
 impl Attributes {
     #[cfg(test)]
     pub fn new() -> Attributes {
-        Attributes([].into())
+        Attributes {
+            attrs: [].into(),
+            id_idx: None,
+            class_idx: None,
+        }
     }
 
     /// Creates an iterable `Attributes` from the C array of borrowed C strings.
@@ -61,6 +64,8 @@ impl Attributes {
         attrs: *const *const libc::c_char,
     ) -> Result<Attributes, LoadingError> {
         let mut array = Vec::with_capacity(n_attributes);
+        let mut id_idx = None;
+        let mut class_idx = None;
 
         if n_attributes > limits::MAX_LOADED_ATTRIBUTES {
             return Err(LoadingError::LimitExceeded(
@@ -102,40 +107,53 @@ impl Attributes {
                     let value_str = str::from_utf8_unchecked(value_slice);
                     let value_atom = DefaultAtom::from(value_str);
 
+                    let idx = array.len() as u16;
+                    match qual_name.expanded() {
+                        expanded_name!("", "id") => id_idx = Some(idx),
+                        expanded_name!("", "class") => class_idx = Some(idx),
+                        _ => (),
+                    }
+
                     array.push((qual_name, value_atom));
                 }
             }
         }
 
-        Ok(Attributes(array.into()))
+        Ok(Attributes {
+            attrs: array.into(),
+            id_idx,
+            class_idx,
+        })
     }
 
     /// Returns the number of attributes.
     pub fn len(&self) -> usize {
-        self.0.len()
+        self.attrs.len()
     }
 
     /// Creates an iterator that yields `(QualName, &'a str)` tuples.
     pub fn iter(&self) -> AttributesIter<'_> {
-        AttributesIter(self.0.iter())
+        AttributesIter(self.attrs.iter())
     }
 
-    /// Creates an iterator that yields `(AttributeIndex, QualName, &'a str)` tuples.
-    pub fn iter_indexed(&self) -> impl Iterator<Item = (AttributeIndex, QualName, &'_ str)> + '_ {
-        self.iter().enumerate().map(|(index, (name, value))| {
-            let index = AttributeIndex(
-                index
-                    .try_into()
-                    .expect("overlong indexes are filtered at creation time"),
-            );
-            (index, name, value)
+    pub fn get_id(&self) -> Option<&str> {
+        self.id_idx.and_then(|idx| {
+            self.attrs
+                .get(usize::from(idx))
+                .map(|(_name, value)| &value[..])
+        })
+    }
+
+    pub fn get_class(&self) -> Option<&str> {
+        self.class_idx.and_then(|idx| {
+            self.attrs
+                .get(usize::from(idx))
+                .map(|(_name, value)| &value[..])
         })
     }
 
-    pub fn get_by_index(&self, idx: AttributeIndex) -> Option<&str> {
-        self.0
-            .get(usize::from(idx.0))
-            .map(|(_name, value)| &value[..])
+    pub fn clear_class(&mut self) {
+        self.class_idx = None;
     }
 }
 
diff --git a/src/xml/mod.rs b/src/xml/mod.rs
index 6a414a736..8a6c7ed40 100644
--- a/src/xml/mod.rs
+++ b/src/xml/mod.rs
@@ -33,7 +33,7 @@ mod attributes;
 mod xml2;
 mod xml2_load;
 
-pub use attributes::{AttributeIndex, Attributes};
+pub use attributes::Attributes;
 
 #[derive(Clone)]
 enum Context {