XML: refactor XML::NodeSet to wrap a Slice(XML::Node)

The main advantage is a much simpler interface. The disadvantages are:

1. `XML::NodeSet` can now only hold 2.1 billion entries because
   `Slice#size` is an Int32; it shouldn't be an issue in practice.

2. `XML::Node#children` will now allocate an `XML::Node` for every child
   node, but will only allocate once, not on every access.

Author: ysbaddaden

src/xml/node.cr

@@ -81,18 +81,21 @@ class XML::Node
   # Gets the list of children for this node as a `XML::NodeSet`.
   def children : XML::NodeSet
     child = @node.value.children
+    return NodeSet.new unless child
 
-    set = LibXML.xmlXPathNodeSetCreate(child)
+    size = 1
+    while child = child.value.next
+      size += 1
+    end
 
-    if child
+    child = @node.value.children
+    nodes = Slice(Node).new(size) do
+      node = Node.new(child, @document)
       child = child.value.next
-      while child
-        LibXML.xmlXPathNodeSetAddUnique(set, child)
-        child = child.value.next
-      end
+      node
     end
 
-    NodeSet.new(@document, set)
+    NodeSet.new(nodes)
   end
 
   # Returns `true` if this is a comment node.

src/xml/node_set.cr

@@ -1,54 +1,33 @@
-class XML::NodeSet
+struct XML::NodeSet
   include Enumerable(Node)
 
   # :nodoc:
-  def initialize(@doc : Node, @xpath_object : LibXML::XPathObject*)
-    @set = @xpath_object.value.nodesetval
-  end
+  def self.new(set : LibXML::NodeSet*, document : Node)
+    return NodeSet.new unless set || set.value.node_nr > 0
 
-  # :nodoc:
-  def initialize(@doc : Node, @set : LibXML::NodeSet*)
-    @xpath_object = Pointer(LibXML::XPathObject).null
-  end
-
-  # :nodoc:
-  def initialize(@doc : Node)
-    @xpath_object = Pointer(LibXML::XPathObject).null
-    @set = LibXML.xmlXPathNodeSetCreate(nil)
+    nodes = Slice(Node).new(set.value.node_nr) do |i|
+      Node.new(set.value.node_tab[i], document)
+    end
+    NodeSet.new(nodes)
   end
 
   # :nodoc:
-  def finalize
-    if @xpath_object.null?
-      LibXML.xmlXPathFreeNodeSet(@set)
-    else
-      LibXML.xmlXPathFreeObject(@xpath_object)
-    end
+  def initialize(nodes : Slice(Node)? = nil)
+    @nodes = nodes || Slice(Node).new(0, Pointer(Void).null.as(Node))
   end
 
-  def [](index : Int) : XML::Node
-    index += size if index < 0
-
-    unless 0 <= index < size
-      raise IndexError.new
-    end
-
-    internal_at(index)
+  def [](index : Int) : Node
+    @nodes[index]
   end
 
   def each(&) : Nil
-    size.times do |i|
-      yield internal_at(i)
-    end
+    @nodes.each { |node| yield node }
   end
 
   def empty? : Bool
-    size == 0
+    @nodes.empty?
   end
 
-  # See `Object#hash(hasher)`
-  def_hash object_id
-
   def inspect(io : IO) : Nil
     io << '['
     join io, ", ", &.inspect(io)
@@ -60,22 +39,10 @@ class XML::NodeSet
   end
 
   def size : Int32
-    @set.value.node_nr
-  end
-
-  def object_id
-    @set.address
+    @nodes.size
   end
 
   def to_s(io : IO) : Nil
     join io, '\n'
   end
-
-  def to_unsafe
-    @set
-  end
-
-  private def internal_at(index)
-    Node.new(@set.value.node_tab[index], @doc)
-  end
 end

src/xml/xpath_context.cr

@@ -42,14 +42,13 @@ class XML::XPathContext
       when LibXML::XPathObjectType::BOOLEAN
         xpath_object.value.boolval != 0
       when LibXML::XPathObjectType::NODESET
-        if xpath_object.value.nodesetval
-          # don't free xpath object: that would free the nodeset
-          return NodeSet.new(@node.document, xpath_object)
-        end
+        NodeSet.new(xpath_object.value.nodesetval, @node.document)
+      else
+        NodeSet.new
       end
 
     LibXML.xmlXPathFreeObject(xpath_object)
-    retval || NodeSet.new(@node.document)
+    retval
   end
 
   def register_namespaces(namespaces) : Nil