[PATCH, applied] Fix recursive array type definition

[PATCH, applied] Fix recursive array type definition

[Dodji Seketeli]

Hello,

This is a follow-up of the patch below:

commit b00ba10e1deae4eae58cb783c01c69f384f392c1
Author: Dodji Seketeli <dodji@redhat.com>
Date:   Sat May 22 01:07:26 2021 +0200

    xml reader: Fix recursive qualified & reference type definition

    This is a followup patch for the fix for
    https://bugzilla.redhat.com/show_bug.cgi?id=1944088, which was in the
    patch:

        commit 51ae965305f9eaa554d5d2111fa24eeb07d35244
        Author: Dodji Seketeli <dodji@redhat.com>
        Date:   Fri May 21 23:55:44 2021 +0200

            abixml reader: Fix recursive type definition handling

This patch basically adjusts build_array_type_def to build the array
type early without trying to create the array element type first.  The
array type is then registered, and then the array element type is
created.  That way, if the element type indirectly needs the array
type being created, then it's going to be used.  Then the element type
is set to the array once it's created.

The patch adjusts the code of the array type to allow creating the
array without element types and then setting the element type later.

	* include/abg-ir.h (array_type_def::update_size): Declare new
	private member function.
	(array_type_def::array_type_def): Declare ...
	* src/abg-ir.cc (array_type_def::array_type_def): ... a new
	constructor that takes no element type.
	(array_type_def::update_size): Define this helper private member
	function.
	(array_type_def::get_subrange_representation): Adjust for this to
	work when there is no element type setup yet.
	(array_type_def::{set_element_type, append_subranges}): Update the
	size and name of the array.
	* src/abg-reader.cc (build_array_type_def): Create the array type
	before the element type so that the later can re-use the former.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
Applied to master.
---
 include/abg-ir.h  |  6 ++++
 src/abg-ir.cc     | 86 ++++++++++++++++++++++++++++++++++++++---------
 src/abg-reader.cc | 52 +++++++++++++---------------
 3 files changed, 100 insertions(+), 44 deletions(-)

diff --git a/include/abg-ir.h b/include/abg-ir.h
index 189ba560..da43727f 100644
--- a/include/abg-ir.h
+++ b/include/abg-ir.h
@@ -2399,6 +2399,8 @@ private:
   // Forbidden.
   array_type_def();
 
+  void update_size();
+
 public:
 
   /// Hasher for intances of array_type_def.
@@ -2544,6 +2546,10 @@ public:
 		 const std::vector<subrange_sptr>& subs,
 		 const location& locus);
 
+  array_type_def(environment* env,
+		 const std::vector<subrange_sptr>& subs,
+		 const location& locus);
+
   translation_unit::language
   get_language() const;
 
diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index a463169e..2de7accc 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -16290,6 +16290,62 @@ array_type_def::array_type_def(const type_base_sptr			e_type,
   append_subranges(subs);
 }
 
+/// Constructor for the type array_type_def
+///
+/// This constructor builds a temporary array that has no element type
+/// associated.  Later when the element type is available, it be set
+/// with the array_type_def::set_element_type() member function.
+///
+/// Note how the constructor expects a vector of subrange
+/// objects. Parsing of the array information always entails
+/// parsing the subrange info as well, thus the class subrange_type
+/// is defined inside class array_type_def and also parsed
+/// simultaneously.
+///
+/// @param env the environment of the array type.
+///
+/// @param subs a vector of the array's subranges(dimensions)
+///
+/// @param locus the source location of the array type definition.
+array_type_def::array_type_def(environment*				env,
+			       const std::vector<subrange_sptr>&	subs,
+			       const location&				locus)
+  : type_or_decl_base(env,
+		      ARRAY_TYPE
+		      | ABSTRACT_TYPE_BASE
+		      | ABSTRACT_DECL_BASE),
+    type_base(env, 0, 0),
+    decl_base(env, locus),
+    priv_(new priv)
+{
+  runtime_type_instance(this);
+  append_subranges(subs);
+}
+
+/// Update the size of the array.
+///
+/// This function computes the size of the array and sets it using
+/// type_base::set_size_in_bits().
+void
+array_type_def::update_size()
+{
+  type_base_sptr e = priv_->element_type_.lock();
+  if (e)
+    {
+      size_t s = e->get_size_in_bits();
+      if (s)
+	{
+	  for (const auto &sub : get_subranges())
+	    s *= sub->get_length();
+
+	  const environment* env = e->get_environment();
+	  ABG_ASSERT(env);
+	  set_size_in_bits(s);
+	}
+      set_alignment_in_bits(e->get_alignment_in_bits());
+    }
+}
+
 string
 array_type_def::get_subrange_representation() const
 {
@@ -16318,15 +16374,21 @@ get_type_representation(const array_type_def& a, bool internal)
       o << "array ("
 	<< a.get_subrange_representation()
 	<< ") of "
-	<<  e_type->get_pretty_representation(internal);
+	<<  e_type ? e_type->get_pretty_representation(internal):string("void");
     }
   else
     {
       if (internal)
-	r = get_type_name(e_type, /*qualified=*/true, /*internal=*/true)
+	r = (e_type
+	     ? get_type_name(e_type,
+			     /*qualified=*/true,
+			     /*internal=*/true)
+	     : string("void"))
 	  + a.get_subrange_representation();
       else
-	r = get_type_name(e_type, /*qualified=*/false, /*internal=*/false)
+	r = (e_type
+	     ? get_type_name(e_type, /*qualified=*/false, /*internal=*/false)
+	     : string("void"))
 	  + a.get_subrange_representation();
     }
 
@@ -16509,6 +16571,8 @@ void
 array_type_def::set_element_type(const type_base_sptr& element_type)
 {
   priv_->element_type_ = element_type;
+  update_size();
+  set_name(get_environment()->intern(get_pretty_representation()));
 }
 
 /// Append subranges from the vector @param subs to the current
@@ -16516,20 +16580,12 @@ array_type_def::set_element_type(const type_base_sptr& element_type)
 void
 array_type_def::append_subranges(const std::vector<subrange_sptr>& subs)
 {
-  size_t s = get_element_type()->get_size_in_bits();
 
-  for (std::vector<shared_ptr<subrange_type> >::const_iterator i = subs.begin();
-       i != subs.end();
-       ++i)
-    {
-      priv_->subranges_.push_back(*i);
-      s *= (*i)->get_length();
-    }
+  for (const auto &sub : subs)
+    priv_->subranges_.push_back(sub);
 
-  const environment* env = get_environment();
-  ABG_ASSERT(env);
-  set_name(env->intern(get_pretty_representation()));
-  set_size_in_bits(s);
+  update_size();
+  set_name(get_environment()->intern(get_pretty_representation()));
 }
 
 /// @return true if one of the sub-ranges of the array is infinite, or
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index e63018a6..e2286672 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -4118,6 +4118,18 @@ build_array_type_def(read_context&	ctxt,
       return result;
     }
 
+  string id;
+  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "id"))
+    id = CHAR_STR(s);
+  ABG_ASSERT(!id.empty());
+
+  if (type_base_sptr d = ctxt.get_type_decl(id))
+    {
+      array_type_def_sptr ty = is_array_type(d);
+      ABG_ASSERT(ty);
+      return ty;
+    }
+
   int dimensions = 0;
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "dimensions"))
     dimensions = atoi(CHAR_STR(s));
@@ -4126,11 +4138,6 @@ build_array_type_def(read_context&	ctxt,
   if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "type-id"))
     type_id = CHAR_STR(s);
 
-  // The type of array elements.
-  type_base_sptr type =
-    ctxt.build_or_get_type_decl(type_id, true);
-  ABG_ASSERT(type);
-
   // maybe building the type of array elements triggered building this
   // one in the mean time ...
   if (decl_base_sptr d = ctxt.get_decl_for_xml_node(node))
@@ -4165,20 +4172,6 @@ build_array_type_def(read_context&	ctxt,
 	return nil;
     }
 
-  string id;
-  if (xml_char_sptr s = XML_NODE_GET_ATTRIBUTE(node, "id"))
-    id = CHAR_STR(s);
-  ABG_ASSERT(!id.empty());
-
-  if (type_base_sptr d = ctxt.get_type_decl(id))
-    {
-      array_type_def_sptr ty = is_array_type(d);
-      ABG_ASSERT(ty);
-      ABG_ASSERT(*type == *ty->get_element_type());
-      ABG_ASSERT(type->get_alignment_in_bits() == alignment_in_bits);
-      return ty;
-    }
-
   location loc;
   read_location(ctxt, node, loc);
   array_type_def::subranges_type subranges;
@@ -4200,10 +4193,17 @@ build_array_type_def(read_context&	ctxt,
 	  }
       }
 
-  array_type_def_sptr ar_type(new array_type_def(type,
-						 subranges,
-						 loc));
+  array_type_def_sptr ar_type(new array_type_def(ctxt.get_environment(),
+						 subranges, loc));
   maybe_set_artificial_location(ctxt, node, ar_type);
+  if (ctxt.push_and_key_type_decl(ar_type, id, add_to_current_scope))
+    ctxt.map_xml_node_to_decl(node, ar_type);
+
+  // The type of array elements.
+  type_base_sptr type =
+    ctxt.build_or_get_type_decl(type_id, true);
+  ABG_ASSERT(type);
+  ar_type->set_element_type(type);
 
   if (dimensions != ar_type->get_dimension_count()
       || (alignment_in_bits
@@ -4245,13 +4245,7 @@ build_array_type_def(read_context&	ctxt,
 	}
     }
 
-  if (ctxt.push_and_key_type_decl(ar_type, id, add_to_current_scope))
-    {
-      ctxt.map_xml_node_to_decl(node, ar_type);
-      return ar_type;
-    }
-
-  return nil;
+  return ar_type;
 }
 
 /// Build an @ref enum_type_decl from the XML node that represents it,
-- 
2.30.0


-- 
		Dodji