Skip to content

Extract XML::Document from XML::Node#15920

Merged
straight-shoota merged 2 commits intocrystal-lang:masterfrom
ysbaddaden:feature/add-xml-document-type
Jul 2, 2025
Merged

Extract XML::Document from XML::Node#15920
straight-shoota merged 2 commits intocrystal-lang:masterfrom
ysbaddaden:feature/add-xml-document-type

Conversation

@ysbaddaden
Copy link
Collaborator

@ysbaddaden ysbaddaden commented Jun 23, 2025

Introduces the XML::Document type to wrap a LibXML::Doc*, while XML::Node is meant to wrap the subtree LibXML::Node*.

The XML::Document inherits from XML::Node and acts exactly like a XML::Node, so it should hopefully be a transparent change (no breaking changes).

The main advantage is that we now register a single finalizer instead of one for every node (only the document needed a finalizer).

Sadly, it doesn't allow to have a dual strong reference from a document to its nodes, and from the nodes to their document. Boehm GC still complains about cyclic finalization, despite only the document having a finalizer.

Follow up to #15906
Related to #15915

Introduces the XML::Document type to wrap a Pointer(LibXML::Doc), while
XLM::Node is meant to wrap the subtree Pointer(LibXML::Node). The
XML::Document inherits from XML::Node and acts exactly like a XML::Node,
so it should hopefully be a transparent change.

The main advantage is that we now register a single finalizer instead of
one for every node (only the document needed a finalizer). Sadly, it
doesn't allow to have a dual strong reference from a document to its
nodes, and from the nodes to their document. Boehm GC still complains
about cyclic finalization, despite only the document having a
finalizer.
@ysbaddaden ysbaddaden force-pushed the feature/add-xml-document-type branch from c5791a6 to 210a1c8 Compare June 26, 2025 16:43
@ysbaddaden ysbaddaden marked this pull request as ready for review June 26, 2025 16:45
@straight-shoota straight-shoota changed the title Extract XML::Document from XML::Node Extract XML::Document from XML::Node Jun 27, 2025
# reference of its XML::Node and any instantiated descendant must be updated
# to pointer to the new document Node.
@document : Node
@document : Document
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: could use a getter.

@straight-shoota straight-shoota added this to the 1.17.0 milestone Jul 1, 2025
@straight-shoota straight-shoota merged commit 1edc082 into crystal-lang:master Jul 2, 2025
42 checks passed
@ysbaddaden ysbaddaden deleted the feature/add-xml-document-type branch July 3, 2025 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants