Re: [PATCH] XML reader: improve XML node traversal

[Giuliano Procida <gprocida@google.com>]

Thanks for the review.

Each of the long lines was a non-standard iteration over nodes and actually
deserved better.

On Wed, 31 Mar 2021 at 15:24, Matthias Maennich <maennich@google.com> wrote:

> On Wed, Mar 31, 2021 at 10:23:51AM +0100, Giuliano Procida wrote:
> >Bug 27616: libabigail treats empty and nearly empty XML elements
> inconsistently
> >
> >The XML reader uses libxml's Reader API for parsing XML and traverses
> >high-level XML nodes using reader cursor advancement. Low-level nodes
> >are read into complete node trees and are traversed following
> >children, next and (in one instance) parent pointers. Some
> >intermediate-level nodes can be parsed both ways.
> >
> >A lot of code cares about node types, typically skipping over
> >non-element nodes. Unfortunately, in a few places, the tracked "corpus
> >node" needs to land on text (whitespace) within elements for the XML
> >reader to work; stripping text nodes out causes some elements to be
> >skipped. Almost all node tree traversals are very straightforward and
> >can be expressed using simpler primitves than at present.
> >
> >This commit changes all the node tree traversals to use two node
> >iteration primitives consistently. These are "first_child" and
> >"next_child", both of which automatically skip text nodes. They
> >replace the existing "advance_to_next_sibling_element" and *all*
> >direct references to "children" and "next" nodes. The unwanted
> >dependency on text within elements has been fixed.
> >
> >For-loops constructed with the new primitives present only element
> >nodes to the loop body. This allows a lot of redundant checking for
> >null / non-element nodes to be removed.
> >
> >These changes have been tested by using abidiff to comparing all
> >current test XML files with copies where all text has been removed.
> >One such test case is included in this commit.
> >
> >       * include/abg-libxml-utils.h
> >       (advance_to_next_sibling_element): Remove this function.
> >       (first_child): Add new function. (next_child): Likewise. This
> >       is simpler than but functionally identical to
> >       advance_to_next_sibling_element.
> >       * src/abg-libxml-utils.cc (skip_non_elements): New function to
> >       skip non-element nodes in a sibling node list. (first_child):
> >       New function to find the first child element of a node.
> >       (next_child): New function to find the next sibling element of
> >       a node.
> >       * src/abg-reader.cc (walk_xml_node_to_map_type_ids): Remove
> >       redundant node checks. Iterate over children with first_child
> >       and next_child helpers. (read_translation_unit): Likewise.
> >       (read_translation_unit_from_input): Remove a no-longer-needed
> >       ->next and redundant node checks. Advance "corpus node" after
> >       reading the translation unit. (read_symbol_db_from_input):
> >       Remove a no-longer-needed ->next and advance "corpus node" as
> >       part of for-loop.  Remove redundant node checks.
> >       (build_needed): Remove redundant node checks. Iterate over
> >       children with first_child and next_child.
> >       (read_elf_needed_from_input): Remove redundant node checks and
> >       simplify logic. Move to next element node after reading
> >       elf-needed node. (read_corpus_from_input): Remove no-children
> >       early return (which was inhibited by text nodes anyway). Set
> >       initial "corpus node" using first_child instead of children
> >       pointer. Remove redundant "corpus node" updates at end of
> >       function as the caller takes care of this and the node can
> >       become null with the new primitives.
> >       (read_corpus_group_from_input): Iterate over children using
> >       first_child and next child. Set "corpus node" for each corpus,
> >       not just the first. Incidentally keep going even if one corpus
> >       parse fails. (build_namespace_decl): Remove redundant node
> >       checks. Iterate over children using first_child and
> >       next_child. (build_elf_symbol): Remove redundant node checks.
> >       (build_elf_symbol_db): Iterate over children using first_child
> >       and next_child. (build_function_decl): Remove redundant node
> >       checks. Iterate over children using first_child and
> >       next_child. (build_function_type): Likewise.
> >       (build_array_type_def): Likewise. (build_enum_type_decl):
> >       Likewise. (build_class_decl): Likewise. (build_union_decl):
> >       Likewise. (build_function_tdecl): Likewise.
> >       (build_type_composition): Likewise.
> >       (build_template_tparameter): Likewise.
> >       (tests/data/Makefile.am): Add new test files.
> >       (tests/data/test-abidiff-exit/test-squished-report.txt): Add
> >       empty diff report.
> >       (tests/data/test-abidiff-exit/test-squished-v0.abi): Add test
> >       ABI file, containing all ELF elements.
> >       (tests/data/test-abidiff-exit/test-squished-v1.abi): Likewise
> >       and identical, except that all text has been stripped.
> >       (tests/test-abidiff-exit.cc): Add new test case.
> >
> >Signed-off-by: Giuliano Procida <gprocida@google.com>
>
> That is a very nice simplification that comes with this fix! Thanks!
>
>
Cheers.

v2 on the way.

Giuliano.


> Reviewed-by: Matthias Maennich <maennich@google.com>
>
> Cheers,
> Matthias
>
> >---
> > include/abg-libxml-utils.h                    |   5 +-
> > src/abg-libxml-utils.cc                       |  47 ++--
> > src/abg-reader.cc                             | 208 +++++-------------
> > tests/data/Makefile.am                        |   3 +
> > .../test-squished-report.txt                  |   0
> > .../test-abidiff-exit/test-squished-v0.abi    |  43 ++++
> > .../test-abidiff-exit/test-squished-v1.abi    |   1 +
> > tests/test-abidiff-exit.cc                    |  11 +
> > 8 files changed, 136 insertions(+), 182 deletions(-)
> > create mode 100644 tests/data/test-abidiff-exit/test-squished-report.txt
> > create mode 100644 tests/data/test-abidiff-exit/test-squished-v0.abi
> > create mode 100644 tests/data/test-abidiff-exit/test-squished-v1.abi
> >
> >diff --git a/include/abg-libxml-utils.h b/include/abg-libxml-utils.h
> >index 69e940f6..f87ee5a0 100644
> >--- a/include/abg-libxml-utils.h
> >+++ b/include/abg-libxml-utils.h
> >@@ -82,7 +82,10 @@ int get_xml_node_depth(xmlNodePtr);
> >   reinterpret_cast<char*>(xml_char_str.get())
> >
> > xmlNodePtr
> >-advance_to_next_sibling_element(xmlNodePtr node);
> >+first_child(xmlNodePtr node);
> >+
> >+xmlNodePtr
> >+next_child(xmlNodePtr node);
> >
> > void
> > escape_xml_string(const std::string& str,
> >diff --git a/src/abg-libxml-utils.cc b/src/abg-libxml-utils.cc
> >index a1acff1f..baea25ac 100644
> >--- a/src/abg-libxml-utils.cc
> >+++ b/src/abg-libxml-utils.cc
> >@@ -413,40 +413,39 @@ unescape_xml_comment(const std::string& str)
> >   return result;
> > }
> >
> >-/// Maybe get the next sibling element node of an XML node, or stay to
> the sam
> >+/// Skip until we reach an XML element node or run out of (child) nodes.
> > ///
> >-/// If there is no next sibling xml element node, the function returns
> >-/// the initial node.
> >+/// @param a pointer to a node.
> > ///
> >-/// @param node the initial node to consider.
> >-///
> >-/// @return the next sibling node or the initial node @p node.
> >-static xmlNodePtr
> >-go_to_next_sibling_element_or_stay(xmlNodePtr node)
> >+/// @return a pointer to an XML element node or nil.
> >+xmlNodePtr
> >+skip_non_elements(xmlNodePtr node)
> > {
> >-  xmlNodePtr n;
> >-  for (n = node; n; n = n->next)
> >-    {
> >-      if (n->type == XML_ELEMENT_NODE)
> >-      break;
> >-    }
> >-  return n ? n : node;
> >+  while (node && node->type != XML_ELEMENT_NODE)
> >+    node = node->next;
> >+  return node;
> > }
> >
> >-/// Get the next sibling element node of an XML node.
> >+/// Get the first child element of an XML node.
> > ///
> >-/// If there is no next sibling xml element node, the function returns
> nil.
> >+/// @param a pointer to the node whose children are to be perused.
> > ///
> >-/// @param node the XML node to consider.
> >+/// @return a pointer to the first XML element child of @p node or nil.
> >+xmlNodePtr
> >+first_child(xmlNodePtr node)
> >+{
> >+  return skip_non_elements(node->children);
> >+}
> >+
> >+/// Get the next sibling element of an XML node.
> > ///
> >-/// @return the next sibling element node or nil.
> >+/// @param a pointer to the XML node whose following siblings are to be
> perused.
> >+///
> >+/// @return a pointer to the next sibling element of @p node or nil.
> > xmlNodePtr
> >-advance_to_next_sibling_element(xmlNodePtr node)
> >+next_child(xmlNodePtr node)
> > {
> >-  xmlNodePtr n = go_to_next_sibling_element_or_stay(node->next);
> >-  if (n == 0 || n->type != XML_ELEMENT_NODE)
> >-    return 0;
> >-  return n;
> >+  return skip_non_elements(node->next);
> > }
> >
> > }//end namespace xml
> >diff --git a/src/abg-reader.cc b/src/abg-reader.cc
> >index 3e552864..a143eb58 100644
> >--- a/src/abg-reader.cc
> >+++ b/src/abg-reader.cc
> >@@ -1343,18 +1343,13 @@ static void
> > walk_xml_node_to_map_type_ids(read_context& ctxt,
> >                             xmlNodePtr node)
> > {
> >-  xmlNodePtr n = node;
> >-
> >-  if (!n || n->type != XML_ELEMENT_NODE)
> >-    return;
> >-
> >-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(n, "id"))
> >+  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "id"))
> >     {
> >       string id = CHAR_STR(s);
> >-      ctxt.map_id_and_node(id, n);
> >+      ctxt.map_id_and_node(id, node);
> >     }
> >
> >-  for (n = n->children; n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     walk_xml_node_to_map_type_ids(ctxt, n);
> > }
> >
> >@@ -1396,12 +1391,8 @@ read_translation_unit(read_context& ctxt,
> translation_unit& tu, xmlNodePtr node)
> >       || !ctxt.get_corpus())
> >     walk_xml_node_to_map_type_ids(ctxt, node);
> >
> >-  for (xmlNodePtr n = node->children; n; n = n->next)
> >-    {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-      handle_element_node(ctxt, n, /*add_decl_to_scope=*/true);
> >-    }
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >+    handle_element_node(ctxt, n, /*add_decl_to_scope=*/true);
> >
> >   ctxt.pop_scope_or_abort(tu.get_global_scope());
> >
> >@@ -1495,16 +1486,9 @@ read_translation_unit_from_input(read_context&
> ctxt)
> >   else
> >     {
> >       node = 0;
> >-      for (xmlNodePtr n = ctxt.get_corpus_node()->next; n; n = n->next)
> >-      {
> >-        if (!n
> >-            || n->type != XML_ELEMENT_NODE)
> >-          continue;
> >-        if (!xmlStrEqual(n->name, BAD_CAST("abi-instr")))
> >-          return nil;
> >-        node = n;
> >-        break;
> >-      }
> >+      xmlNodePtr n = ctxt.get_corpus_node();
> >+      if (n && xmlStrEqual(n->name, BAD_CAST("abi-instr")))
> >+      node = n;
> >     }
> >
> >   if (node == 0)
> >@@ -1517,9 +1501,9 @@ read_translation_unit_from_input(read_context&
>  ctxt)
> >   // case, after that unexpected call to read_translation_unit(), the
> >   // current corpus node of the context is going to point to that
> >   // translation unit that has been read under the hood.  Let's set
> >-  // the corpus node to the one we initially called
> >+  // the corpus node to just after the one we initially called
> >   // read_translation_unit() on here.
> >-  ctxt.set_corpus_node(node);
> >+  ctxt.set_corpus_node(xml::next_child(node));
> >   return tu;
> > }
> >
> >@@ -1593,11 +1577,8 @@ read_symbol_db_from_input(read_context&
> ctxt,
> >       xmlTextReaderNext(reader.get());
> >       }
> >   else
> >-    for (xmlNodePtr n = ctxt.get_corpus_node()->next; n; n = n->next)
> >+    for (xmlNodePtr n = ctxt.get_corpus_node(); n; n =
> xml::next_child(n), ctxt.set_corpus_node(n))
>
> That line looks a little long.
>
>
It makes sense to only update the "corpus node" once, after the loop
completes. I've restructured it and indentation has been reduced as a side
effect.


> >       {
> >-      if (!n || n->type != XML_ELEMENT_NODE)
> >-        continue;
> >-
> >       bool has_fn_syms = false, has_var_syms = false;
> >       if (xmlStrEqual(n->name, BAD_CAST("elf-function-symbols")))
> >         has_fn_syms = true;
> >@@ -1605,7 +1586,6 @@ read_symbol_db_from_input(read_context&
>  ctxt,
> >         has_var_syms = true;
> >       else
> >         break;
> >-      ctxt.set_corpus_node(n);
> >       if (has_fn_syms)
> >         {
> >           fn_symdb = build_elf_symbol_db(ctxt, n, true);
> >@@ -1636,16 +1616,12 @@ read_symbol_db_from_input(read_context&
>        ctxt,
> > static bool
> > build_needed(xmlNode* node, vector<string>& needed)
> > {
> >-  if (!node)
> >-    return false;
> >-
> >-  if (!node || !xmlStrEqual(node->name,BAD_CAST("elf-needed")))
> >+  if (!xmlStrEqual(node->name,BAD_CAST("elf-needed")))
> >     return false;
> >
> >-  for (xmlNodePtr n = node->children; n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >-      if (n->type != XML_ELEMENT_NODE
> >-        || !xmlStrEqual(n->name, BAD_CAST("dependency")))
> >+      if (!xmlStrEqual(n->name, BAD_CAST("dependency")))
> >       continue;
> >
> >       string name;
> >@@ -1693,27 +1669,19 @@ read_elf_needed_from_input(read_context&
>  ctxt,
> >       return false;
> >
> >       node = xmlTextReaderExpand(reader.get());
> >-      if (!node)
> >-      return false;
> >     }
> >   else
> >     {
> >-      for (xmlNodePtr n = ctxt.get_corpus_node()->next; n; n = n->next)
> >-      {
> >-        if (!n || n->type != XML_ELEMENT_NODE)
> >-          continue;
> >-        if (!xmlStrEqual(n->name, BAD_CAST("elf-needed")))
> >-          return false;
> >-        node = n;
> >-        break;
> >-      }
> >+      xmlNodePtr n = ctxt.get_corpus_node();
> >+      if (n && xmlStrEqual(n->name, BAD_CAST("elf-needed")))
> >+      node = n;
> >     }
> >
> >   bool result = false;
> >   if (node)
> >     {
> >       result = build_needed(node, needed);
> >-      ctxt.set_corpus_node(node);
> >+      ctxt.set_corpus_node(xml::next_child(node));
> >     }
> >
> >   return result;
> >@@ -1917,10 +1885,7 @@ read_corpus_from_input(read_context& ctxt)
> >       corp.set_soname(reinterpret_cast<char*>(soname_str.get()));
> >     }
> >
> >-  if (!node->children)
> >-    return nil;
> >-
> >-  ctxt.set_corpus_node(node->children);
> >+  ctxt.set_corpus_node(xml::first_child(node));
> >
> >   corpus& corp = *ctxt.get_corpus();
> >
> >@@ -1986,17 +1951,6 @@ read_corpus_from_input(read_context& ctxt)
> >       // call at the beginning of the function.
> >       xmlTextReaderNext(reader.get());
> >     }
> >-  else
> >-    {
> >-      node = ctxt.get_corpus_node();
> >-      node = xml::advance_to_next_sibling_element(node);
> >-      if (!node)
> >-      {
> >-        node = ctxt.get_corpus_node();
> >-        node = xml::advance_to_next_sibling_element(node->parent);
> >-      }
> >-      ctxt.set_corpus_node(node);
> >-    }
> >
> >   return ctxt.get_corpus();
> > }
> >@@ -2046,13 +2000,13 @@ read_corpus_group_from_input(read_context& ctxt)
> >   if (!node)
> >     return nil;
> >
> >-  //node = xml::get_first_element_sibling_if_text(node->children);
> >-  node = xml::advance_to_next_sibling_element(node->children);
> >-  ctxt.set_corpus_node(node);
> >-
> >-  corpus_sptr corp;
> >-  while ((corp = read_corpus_from_input(ctxt)))
> >-    ctxt.get_corpus_group()->add_corpus(corp);
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >+    {
> >+      ctxt.set_corpus_node(n);
> >+      corpus_sptr corp = read_corpus_from_input(ctxt);
> >+      if (corp)
> >+      ctxt.get_corpus_group()->add_corpus(corp);
> >+    }
> >
> >   xmlTextReaderNext(reader.get());
> >
> >@@ -2738,12 +2692,8 @@ build_namespace_decl(read_context&      ctxt,
> >   ctxt.push_decl_to_current_scope(decl, add_to_current_scope);
> >   ctxt.map_xml_node_to_decl(node, decl);
> >
> >-  for (xmlNodePtr n = node->children; n; n = n->next)
> >-    {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-      handle_element_node(ctxt, n, /*add_to_current_scope=*/true);
> >-    }
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >+    handle_element_node(ctxt, n, /*add_to_current_scope=*/true);
> >
> >   ctxt.pop_scope_or_abort(decl);
> >
> >@@ -2767,9 +2717,7 @@ build_elf_symbol(read_context& ctxt, const
> xmlNodePtr node,
> > {
> >   elf_symbol_sptr nil;
> >
> >-  if (!node
> >-      || node->type != XML_ELEMENT_NODE
> >-      || !xmlStrEqual(node->name, BAD_CAST("elf-symbol")))
> >+  if (!xmlStrEqual(node->name, BAD_CAST("elf-symbol")))
> >     return nil;
> >
> >   string name;
> >@@ -2928,7 +2876,7 @@ build_elf_symbol_db(read_context& ctxt,
> >   xml_node_ptr_elf_symbol_sptr_map_type xml_node_ptr_elf_symbol_map;
> >
> >   elf_symbol_sptr sym;
> >-  for (xmlNodePtr n = node->children; n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >       if ((sym = build_elf_symbol(ctxt, n, /*drop_if_suppress=*/true)))
> >       {
> >@@ -3094,12 +3042,9 @@ build_function_decl(read_context&       ctxt,
> >   std::vector<function_decl::parameter_sptr> parms;
> >   type_base_sptr return_type = env->get_void_type();
> >
> >-  for (xmlNodePtr n = node->children; n ; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >-      else if (xmlStrEqual(n->name, BAD_CAST("parameter")))
> >+      if (xmlStrEqual(n->name, BAD_CAST("parameter")))
> >       {
> >         if (function_decl::parameter_sptr p =
> >             build_function_parameter(ctxt, n))
> >@@ -3789,12 +3734,9 @@ build_function_type(read_context&       ctxt,
> >   ctxt.get_translation_unit()->bind_function_type_life_time(fn_type);
> >   ctxt.key_type_decl(fn_type, id);
> >
> >-  for (xmlNodePtr n = node->children; n ; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >-      else if (xmlStrEqual(n->name, BAD_CAST("parameter")))
> >+      if (xmlStrEqual(n->name, BAD_CAST("parameter")))
> >       {
> >         if (function_decl::parameter_sptr p =
> >             build_function_parameter(ctxt, n))
> >@@ -4025,12 +3967,9 @@ build_array_type_def(read_context&      ctxt,
> >   read_location(ctxt, node, loc);
> >   array_type_def::subranges_type subranges;
> >
> >-  for (xmlNodePtr n = node->children; n ; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >-      else if (xmlStrEqual(n->name, BAD_CAST("subrange")))
> >+      if (xmlStrEqual(n->name, BAD_CAST("subrange")))
> >       {
> >         if (array_type_def::subrange_sptr s =
> >             build_subrange_type(ctxt, n))
> >@@ -4183,11 +4122,8 @@ build_enum_type_decl(read_context&      ctxt,
> >
> >   string base_type_id;
> >   enum_type_decl::enumerators enums;
> >-  for (xmlNodePtr n = node->children; n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >       if (xmlStrEqual(n->name, BAD_CAST("underlying-type")))
> >       {
> >         xml_char_sptr a = xml::build_sptr(xmlGetProp(n,
> BAD_CAST("type-id")));
> >@@ -4554,11 +4490,8 @@ build_class_decl(read_context&          ctxt,
> >       decl->set_naming_typedef(naming_typedef);
> >     }
> >
> >-  for (xmlNodePtr n = node->children; !is_decl_only && n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); !is_decl_only && n; n =
> xml::next_child(n))
>
> Likewise.
>
>
I've written a very brief note at
https://sourceware.org/bugzilla/show_bug.cgi?id=26591#c6 and made the
skipping of child elements much more obvious.

(And the same for the other case.)


> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >       if (xmlStrEqual(n->name, BAD_CAST("base-class")))
> >       {
> >         access_specifier access =
> >@@ -4605,11 +4538,8 @@ build_class_decl(read_context&          ctxt,
> >
> >         ctxt.map_xml_node_to_decl(n, decl);
> >
> >-        for (xmlNodePtr p = n->children; p; p = p->next)
> >+        for (xmlNodePtr p = xml::first_child(n); p; p =
> xml::next_child(p))
> >           {
> >-            if (p->type != XML_ELEMENT_NODE)
> >-              continue;
> >-
> >             if (type_base_sptr t =
> >                 build_type(ctxt, p, /*add_to_current_scope=*/true))
> >               {
> >@@ -4643,11 +4573,8 @@ build_class_decl(read_context&          ctxt,
> >         bool is_static = false;
> >         read_static(n, is_static);
> >
> >-        for (xmlNodePtr p = n->children; p; p = p->next)
> >+        for (xmlNodePtr p = xml::first_child(n); p; p =
> xml::next_child(p))
> >           {
> >-            if (p->type != XML_ELEMENT_NODE)
> >-              continue;
> >-
> >             if (var_decl_sptr v =
> >                 build_var_decl(ctxt, p, /*add_to_cur_scope=*/false))
> >               {
> >@@ -4701,11 +4628,8 @@ build_class_decl(read_context&          ctxt,
> >         bool is_ctor = false, is_dtor = false, is_const = false;
> >         read_cdtor_const(n, is_ctor, is_dtor, is_const);
> >
> >-        for (xmlNodePtr p = n->children; p; p = p->next)
> >+        for (xmlNodePtr p = xml::first_child(n); p; p =
> xml::next_child(p))
> >           {
> >-            if (p->type != XML_ELEMENT_NODE)
> >-              continue;
> >-
> >             if (function_decl_sptr f =
> >                 build_function_decl_if_not_suppressed(ctxt, p, decl,
> >
>  /*add_to_cur_sc=*/true))
> >@@ -4740,11 +4664,8 @@ build_class_decl(read_context&          ctxt,
> >         bool is_ctor = false, is_dtor = false, is_const = false;
> >         read_cdtor_const(n, is_ctor, is_dtor, is_const);
> >
> >-        for (xmlNodePtr p = n->children; p; p = p->next)
> >+        for (xmlNodePtr p = xml::first_child(n); p; p =
> xml::next_child(p))
> >           {
> >-            if (p->type != XML_ELEMENT_NODE)
> >-              continue;
> >-
> >             if (shared_ptr<function_tdecl> f =
> >                 build_function_tdecl(ctxt, p,
> >                                      /*add_to_current_scope=*/true))
> >@@ -4954,11 +4875,8 @@ build_union_decl(read_context& ctxt,
> >   ctxt.map_xml_node_to_decl(node, decl);
> >   ctxt.key_type_decl(decl, id);
> >
> >-  for (xmlNodePtr n = node->children; !is_decl_only && n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); !is_decl_only && n; n =
> xml::next_child(n))
>
> Likewise.
>
> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >       if (xmlStrEqual(n->name, BAD_CAST("member-type")))
> >       {
> >         access_specifier access = private_access;
> >@@ -4966,11 +4884,8 @@ build_union_decl(read_context& ctxt,
> >
> >         ctxt.map_xml_node_to_decl(n, decl);
> >
> >-        for (xmlNodePtr p = n->children; p; p = p->next)
> >+        for (xmlNodePtr p = xml::first_child(n); p; p =
> xml::next_child(p))
> >           {
> >-            if (p->type != XML_ELEMENT_NODE)
> >-              continue;
> >-
> >             if (type_base_sptr t =
> >                 build_type(ctxt, p, /*add_to_current_scope=*/true))
> >               {
> >@@ -4998,11 +4913,8 @@ build_union_decl(read_context& ctxt,
> >         bool is_static = false;
> >         read_static(n, is_static);
> >
> >-        for (xmlNodePtr p = n->children; p; p = p->next)
> >+        for (xmlNodePtr p = xml::first_child(n); p; p =
> xml::next_child(p))
> >           {
> >-            if (p->type != XML_ELEMENT_NODE)
> >-              continue;
> >-
> >             if (var_decl_sptr v =
> >                 build_var_decl(ctxt, p, /*add_to_cur_scope=*/false))
> >               {
> >@@ -5040,11 +4952,8 @@ build_union_decl(read_context& ctxt,
> >         bool is_ctor = false, is_dtor = false, is_const = false;
> >         read_cdtor_const(n, is_ctor, is_dtor, is_const);
> >
> >-        for (xmlNodePtr p = n->children; p; p = p->next)
> >+        for (xmlNodePtr p = xml::first_child(n); p; p =
> xml::next_child(p))
> >           {
> >-            if (p->type != XML_ELEMENT_NODE)
> >-              continue;
> >-
> >             if (function_decl_sptr f =
> >                 build_function_decl_if_not_suppressed(ctxt, p, decl,
> >
>  /*add_to_cur_sc=*/true))
> >@@ -5073,11 +4982,8 @@ build_union_decl(read_context& ctxt,
> >         bool is_ctor = false, is_dtor = false, is_const = false;
> >         read_cdtor_const(n, is_ctor, is_dtor, is_const);
> >
> >-        for (xmlNodePtr p = n->children; p; p = p->next)
> >+        for (xmlNodePtr p = xml::first_child(n); p; p =
> xml::next_child(p))
> >           {
> >-            if (p->type != XML_ELEMENT_NODE)
> >-              continue;
> >-
> >             if (function_tdecl_sptr f =
> >                 build_function_tdecl(ctxt, p,
> >                                      /*add_to_current_scope=*/true))
> >@@ -5152,11 +5058,8 @@ build_function_tdecl(read_context& ctxt,
> >   ctxt.push_decl_to_current_scope(fn_tmpl_decl, add_to_current_scope);
> >
> >   unsigned parm_index = 0;
> >-  for (xmlNodePtr n = node->children; n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >       if (template_parameter_sptr parm =
> >         build_template_parameter(ctxt, n, parm_index, fn_tmpl_decl))
> >       {
> >@@ -5216,11 +5119,8 @@ build_class_tdecl(read_context& ctxt,
> >   ctxt.push_decl_to_current_scope(class_tmpl, add_to_current_scope);
> >
> >   unsigned parm_index = 0;
> >-  for (xmlNodePtr n = node->children; n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >       if (template_parameter_sptr parm=
> >         build_template_parameter(ctxt, n, parm_index, class_tmpl))
> >       {
> >@@ -5335,11 +5235,8 @@ build_type_composition(read_context&
> ctxt,
> >
>  ctxt.push_decl_to_current_scope(dynamic_pointer_cast<decl_base>(result),
> >                                 /*add_to_current_scope=*/true);
> >
> >-  for (xmlNodePtr n = node->children; n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >       if ((composed_type =
> >          build_pointer_type_def(ctxt, n,
> >                                 /*add_to_current_scope=*/true))
> >@@ -5465,11 +5362,8 @@ build_template_tparameter(read_context& ctxt,
> >
> >   // Go parse template parameters that are children nodes
> >   int parm_index = 0;
> >-  for (xmlNodePtr n = node->children; n; n = n->next)
> >+  for (xmlNodePtr n = xml::first_child(node); n; n = xml::next_child(n))
> >     {
> >-      if (n->type != XML_ELEMENT_NODE)
> >-      continue;
> >-
> >       if (shared_ptr<template_parameter> p =
> >         build_template_parameter(ctxt, n, parm_index, result))
> >       {
> >diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
> >index 4fb2d6a4..7bdae7bf 100644
> >--- a/tests/data/Makefile.am
> >+++ b/tests/data/Makefile.am
> >@@ -197,6 +197,9 @@ test-abidiff-exit/test-non-leaf-array-v0.o \
> > test-abidiff-exit/test-non-leaf-array-v1.c \
> > test-abidiff-exit/test-non-leaf-array-v1.o \
> > test-abidiff-exit/test-non-leaf-array-report.txt \
> >+test-abidiff-exit/test-squished-v0.abi \
> >+test-abidiff-exit/test-squished-v1.abi \
> >+test-abidiff-exit/test-squished-report.txt \
> > \
> > test-diff-dwarf/test0-v0.cc           \
> > test-diff-dwarf/test0-v0.o                    \
> >diff --git a/tests/data/test-abidiff-exit/test-squished-report.txt
> b/tests/data/test-abidiff-exit/test-squished-report.txt
> >new file mode 100644
> >index 00000000..e69de29b
> >diff --git a/tests/data/test-abidiff-exit/test-squished-v0.abi
> b/tests/data/test-abidiff-exit/test-squished-v0.abi
> >new file mode 100644
> >index 00000000..6b3d0460
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-squished-v0.abi
> >@@ -0,0 +1,43 @@
> >+<abi-corpus version='2.0' path='data/test-read-dwarf/test6.so'>
> >+  <elf-needed>
> >+    <dependency name='libstdc++.so.6'/>
> >+    <dependency name='libm.so.6'/>
> >+    <dependency name='libgcc_s.so.1'/>
> >+    <dependency name='libc.so.6'/>
> >+  </elf-needed>
> >+  <elf-function-symbols>
> >+    <elf-symbol name='_Z3barv' type='func-type' binding='global-binding'
> visibility='default-visibility' is-defined='yes'/>
> >+    <elf-symbol name='_Z4blehv' type='func-type'
> binding='global-binding' visibility='default-visibility' is-defined='yes'/>
> >+    <elf-symbol name='_ZN1B3fooEv' type='func-type'
> binding='weak-binding' visibility='default-visibility' is-defined='yes'/>
> >+    <elf-symbol name='_fini' type='func-type' binding='global-binding'
> visibility='default-visibility' is-defined='yes'/>
> >+    <elf-symbol name='_init' type='func-type' binding='global-binding'
> visibility='default-visibility' is-defined='yes'/>
> >+  </elf-function-symbols>
> >+  <elf-variable-symbols>
> >+    <elf-symbol name='_ZN1CIiE3barE' size='4' type='object-type'
> binding='gnu-unique-binding' visibility='default-visibility'
> is-defined='yes'/>
> >+    <elf-symbol name='_ZZN1B3fooEvE1a' size='4' type='object-type'
> binding='gnu-unique-binding' visibility='default-visibility'
> is-defined='yes'/>
> >+  </elf-variable-symbols>
> >+  <abi-instr address-size='64' path='test6.cc'
> comp-dir-path='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf'
> language='LANG_C_plus_plus'>
> >+    <type-decl name='int' size-in-bits='32' id='type-id-1'/>
> >+    <class-decl name='B' size-in-bits='8' is-struct='yes'
> visibility='default'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='9' column='1' id='type-id-2'>
> >+      <member-function access='public'>
> >+        <function-decl name='foo' mangled-name='_ZN1B3fooEv'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='11' column='1' visibility='default' binding='global'
> size-in-bits='64' elf-symbol-id='_ZN1B3fooEv'>
> >+          <parameter type-id='type-id-3' name='this'
> is-artificial='yes'/>
> >+          <return type-id='type-id-1'/>
> >+        </function-decl>
> >+      </member-function>
> >+    </class-decl>
> >+    <class-decl name='C&lt;int&gt;' size-in-bits='8' is-struct='yes'
> visibility='default'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='26' column='1' id='type-id-4'>
> >+      <data-member access='public' static='yes'>
> >+        <var-decl name='bar' type-id='type-id-1'
> mangled-name='_ZN1CIiE3barE' visibility='default'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='31' column='1' elf-symbol-id='_ZN1CIiE3barE'/>
> >+      </data-member>
> >+    </class-decl>
> >+    <pointer-type-def type-id='type-id-2' size-in-bits='64'
> id='type-id-5'/>
> >+    <qualified-type-def type-id='type-id-5' const='yes' id='type-id-3'/>
> >+    <function-decl name='bar' mangled-name='_Z3barv'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='19' column='1' visibility='default' binding='global'
> size-in-bits='64' elf-symbol-id='_Z3barv'>
> >+      <return type-id='type-id-1'/>
> >+    </function-decl>
> >+    <function-decl name='bleh' mangled-name='_Z4blehv'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='34' column='1' visibility='default' binding='global'
> size-in-bits='64' elf-symbol-id='_Z4blehv'>
> >+      <return type-id='type-id-1'/>
> >+    </function-decl>
> >+  </abi-instr>
> >+</abi-corpus>
> >diff --git a/tests/data/test-abidiff-exit/test-squished-v1.abi
> b/tests/data/test-abidiff-exit/test-squished-v1.abi
> >new file mode 100644
> >index 00000000..3ffa272b
> >--- /dev/null
> >+++ b/tests/data/test-abidiff-exit/test-squished-v1.abi
> >@@ -0,0 +1 @@
> >+<abi-corpus version='2.0'
> path='data/test-read-dwarf/test6.so'><elf-needed><dependency
> name='libstdc++.so.6'/><dependency name='libm.so.6'/><dependency
> name='libgcc_s.so.1'/><dependency
> name='libc.so.6'/></elf-needed><elf-function-symbols><elf-symbol
> name='_Z3barv' type='func-type' binding='global-binding'
> visibility='default-visibility' is-defined='yes'/><elf-symbol
> name='_Z4blehv' type='func-type' binding='global-binding'
> visibility='default-visibility' is-defined='yes'/><elf-symbol
> name='_ZN1B3fooEv' type='func-type' binding='weak-binding'
> visibility='default-visibility' is-defined='yes'/><elf-symbol name='_fini'
> type='func-type' binding='global-binding' visibility='default-visibility'
> is-defined='yes'/><elf-symbol name='_init' type='func-type'
> binding='global-binding' visibility='default-visibility'
> is-defined='yes'/></elf-function-symbols><elf-variable-symbols><elf-symbol
> name='_ZN1CIiE3barE' size='4' type='object-type'
> binding='gnu-unique-binding' visibility='default-visibility'
> is-defined='yes'/><elf-symbol name='_ZZN1B3fooEvE1a' size='4'
> type='object-type' binding='gnu-unique-binding'
> visibility='default-visibility'
> is-defined='yes'/></elf-variable-symbols><abi-instr address-size='64'
> path='test6.cc'
> comp-dir-path='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf'
> language='LANG_C_plus_plus'><type-decl name='int' size-in-bits='32'
> id='type-id-1'/><class-decl name='B' size-in-bits='8' is-struct='yes'
> visibility='default'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='9' column='1' id='type-id-2'><member-function
> access='public'><function-decl name='foo' mangled-name='_ZN1B3fooEv'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='11' column='1' visibility='default' binding='global'
> size-in-bits='64' elf-symbol-id='_ZN1B3fooEv'><parameter
> type-id='type-id-3' name='this' is-artificial='yes'/><return
> type-id='type-id-1'/></function-decl></member-function></class-decl><class-decl
> name='C&lt;int&gt;' size-in-bits='8' is-struct='yes' visibility='default'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='26' column='1' id='type-id-4'><data-member access='public'
> static='yes'><var-decl name='bar' type-id='type-id-1'
> mangled-name='_ZN1CIiE3barE' visibility='default'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='31' column='1'
> elf-symbol-id='_ZN1CIiE3barE'/></data-member></class-decl><pointer-type-def
> type-id='type-id-2' size-in-bits='64' id='type-id-5'/><qualified-type-def
> type-id='type-id-5' const='yes' id='type-id-3'/><function-decl name='bar'
> mangled-name='_Z3barv'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='19' column='1' visibility='default' binding='global'
> size-in-bits='64' elf-symbol-id='_Z3barv'><return
> type-id='type-id-1'/></function-decl><function-decl name='bleh'
> mangled-name='_Z4blehv'
> filepath='/home/skumari/Tasks/source_repo/dodji/libabigail/tests/data/test-read-dwarf/test6.cc'
> line='34' column='1' visibility='default' binding='global'
> size-in-bits='64' elf-symbol-id='_Z4blehv'><return
> type-id='type-id-1'/></function-decl></abi-instr></abi-corpus>
> >\ No newline at end of file
> >diff --git a/tests/test-abidiff-exit.cc b/tests/test-abidiff-exit.cc
> >index 4283e145..f1526b9e 100644
> >--- a/tests/test-abidiff-exit.cc
> >+++ b/tests/test-abidiff-exit.cc
> >@@ -392,6 +392,17 @@ InOutSpec in_out_specs[] =
> >     "data/test-abidiff-exit/test-non-leaf-array-report.txt",
> >     "output/test-abidiff-exit/test-non-leaf-array-report.txt"
> >   },
> >+  {
> >+    "data/test-abidiff-exit/test-squished-v0.abi",
> >+    "data/test-abidiff-exit/test-squished-v1.abi",
> >+    "",
> >+    "",
> >+    "",
> >+    "--harmless",
> >+    abigail::tools_utils::ABIDIFF_OK,
> >+    "data/test-abidiff-exit/test-squished-report.txt",
> >+    "output/test-abidiff-exit/test-squished-report.txt"
> >+  },
> >   {0, 0, 0 ,0, 0, 0, abigail::tools_utils::ABIDIFF_OK, 0, 0}
> > };
> >
> >--
> >2.31.0.291.g576ba9dcdaf-goog
> >
>