diff mbox series

[2/4] abg-reader.cc: Late canonicalise all types.

Message ID 20200505180612.232158-3-gprocida@google.com
State Rejected
Headers
Series Fix incomplete function type bug in abg-reader |

Commit Message

Giuliano Procida May 5, 2020, 6:06 p.m. UTC 
  The XML reader has ability to canonicalise types both immediately
after creation and after a translation unit (or corpus) has been
read. The latter is referred to as late canonicalisation and needs to
be chosen for a variety of reasons.

Early canonicalisation doesn't appear to bring any performance
advantages and doing things with incomplete types is the cause of at
least one diff reporting bug.

	* src/abg-reader.cc: (maybe_canonicalize_type): Rename this
	function to canonicalize_type_later and make it
	unconditionally schedule its type argument for later
	canonicalisation.
	(perform_late_type_canonicalizing): Update doc comment.
	(read_context::build_or_get_type_decl): Substitute call to
	maybe_canonicalize_type with call to canonicalize_type_later.
	(build_function_decl): Ditto.
	(build_class_decl): Ditto.
	(build_union_decl): Ditto.
	(build_class_tdecl): Ditto.
	(build_type_tparameter): Ditto.
	(build_type_composition): Ditto.
	(build_template_tparameter): Ditto.
	(handle_type_decl): Ditto.
	(handle_qualified_type_decl): Ditto.
	(handle_pointer_type_def): Ditto.
	(handle_reference_type_def): Ditto.
	(handle_function_type): Avoid canonicalising null type.
	Substitute call to maybe_canonicalize_type with call to
	canonicalize_type_later.
	(handle_array_type_def): Ditto.
	(handle_enum_type_decl): Substitute call to
	maybe_canonicalize_type with call to canonicalize_type_later.
	(handle_typedef_decl): Ditto.
	(handle_class_decl): Ditto.
	(handle_union_decl): Ditto.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-reader.cc | 104 ++++++++++++++--------------------------------
 1 file changed, 32 insertions(+), 72 deletions(-)
diff mbox series

Patch

diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index 255a200f..a2bd3c5c 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -889,62 +889,21 @@  public:
     clear_decls_stack();
   }
 
-  /// Test if a type should be canonicalized early.  If so,
-  /// canonicalize it right away.  Otherwise, schedule it for late
-  /// canonicalizing; that is, schedule it so that it's going to be
-  /// canonicalized when the translation unit is fully read.
+  /// Schedule all types for canonicalizing once the translation unit
+  /// is fully read.
   ///
   /// @param t the type to consider for canonicalizing.
   void
-  maybe_canonicalize_type(type_base_sptr t,
-			  bool force_delay = false)
+  canonicalize_type_later(type_base_sptr t)
   {
-    if (!t)
-      return;
+    ABG_ASSERT(t);
+    ABG_ASSERT(!t->get_canonical_type());
 
-    if (t->get_canonical_type())
-      return;
+    // Check that class types have been properly added to their contexts.
+    if (class_decl_sptr c = is_class_type(t))
+      ABG_ASSERT(c->get_scope());
 
-    // If this class has some non-canonicalized sub type, then wait
-    // for the when we've read all the translation unit to
-    // canonicalize all of its non-canonicalized sub types and then we
-    // can canonicalize this one.
-    //
-    // Also, if this is a declaration-only class, wait for the end of
-    // the translation unit reading so that we have its definition and
-    // then we'll use that for canonicalizing it.
-    if (!force_delay
-	&& !type_has_non_canonicalized_subtype(t)
-	&& !is_class_type(t)
-	&& !is_union_type(t)
-	&& !is_wip_type(t)
-	// Below are types that *must* be canonicalized only after
-	// they are added to their context; but then this function
-	// might be called to early, before they are actually added to
-	// their context.
-	//
-	// TODO: make sure this function is called after types are
-	// added to their context, so that we can try to
-	// early-canonicalize some of these types, reducing the size
-	// of the set of types to put on the side, waiting for being
-	// canonicalized.
-	&& !is_method_type(t)
-	&& !is_reference_type(t)
-	&& !is_pointer_type(t)
-	&& !is_qualified_type(t)
-	&& !is_typedef(t)
-	&& !is_enum_type(t)
-	&& !is_function_type(t))
-      canonicalize(t);
-    else
-      {
-	// We do not want to try to canonicalize a class type that
-	// hasn't been properly added to its context.
-	if (class_decl_sptr c = is_class_type(t))
-	  ABG_ASSERT(c->get_scope());
-
-	schedule_type_for_late_canonicalizing(t);
-      }
+    schedule_type_for_late_canonicalizing(t);
   }
 
   /// Schedule a type for being canonicalized after the current
@@ -955,9 +914,9 @@  public:
   schedule_type_for_late_canonicalizing(type_base_sptr t)
   {m_types_to_canonicalize.push_back(t);}
 
-  /// Perform the canonicalizing of types that ought to be done after
-  /// the current translation unit is read.  This function is called
-  /// when the current corpus is fully built.
+  /// Perform the canonicalizing of types after the current
+  /// translation unit is read.  This function is called when the
+  /// current corpus is fully built.
   void
   perform_late_type_canonicalizing()
   {
@@ -1413,7 +1372,7 @@  read_context::build_or_get_type_decl(const string& id,
       if (add_decl_to_scope)
 	pop_scope_or_abort(scope);
 
-      maybe_canonicalize_type(t, !add_decl_to_scope);
+      canonicalize_type_later(t);
     }
   return t;
 }
@@ -3217,7 +3176,7 @@  build_function_decl(read_context&	ctxt,
 
   ctxt.get_translation_unit()->bind_function_type_life_time(fn_type);
 
-  ctxt.maybe_canonicalize_type(fn_type, !add_to_current_scope);
+  ctxt.canonicalize_type_later(fn_type);
 
   ctxt.maybe_add_fn_to_exported_decls(fn_decl.get());
 
@@ -4653,7 +4612,7 @@  build_class_decl(read_context&		ctxt,
 		  decl_base_sptr td = get_type_declaration(t);
 		  ABG_ASSERT(td);
 		  set_member_access_specifier(td, access);
-		  ctxt.maybe_canonicalize_type(t, !add_to_current_scope);
+		  ctxt.canonicalize_type_later(t);
 		  xml_char_sptr i= XML_NODE_GET_ATTRIBUTE(p, "id");
 		  string id = CHAR_STR(i);
 		  ABG_ASSERT(!id.empty());
@@ -5012,7 +4971,7 @@  build_union_decl(read_context& ctxt,
 		  decl_base_sptr td = get_type_declaration(t);
 		  ABG_ASSERT(td);
 		  set_member_access_specifier(td, access);
-		  ctxt.maybe_canonicalize_type(t, !add_to_current_scope);
+		  ctxt.canonicalize_type_later(t);
 		  xml_char_sptr i= XML_NODE_GET_ATTRIBUTE(p, "id");
 		  string id = CHAR_STR(i);
 		  ABG_ASSERT(!id.empty());
@@ -5268,7 +5227,7 @@  build_class_tdecl(read_context&	ctxt,
 						  add_to_current_scope))
 	{
 	  if (c->get_scope())
-	    ctxt.maybe_canonicalize_type(c, /*force_delay=*/false);
+	    ctxt.canonicalize_type_later(c);
 	  class_tmpl->set_pattern(c);
 	}
     }
@@ -5335,7 +5294,7 @@  build_type_tparameter(read_context&		ctxt,
 
   ABG_ASSERT(result->get_environment());
 
-  ctxt.maybe_canonicalize_type(result, /*force_delay=*/false);
+  ctxt.canonicalize_type_later(result);
 
   return result;
 }
@@ -5389,8 +5348,7 @@  build_type_composition(read_context&		ctxt,
 	      build_qualified_type_decl(ctxt, n,
 					/*add_to_current_scope=*/true)))
 	{
-	  ctxt.maybe_canonicalize_type(composed_type,
-				       /*force_delay=*/true);
+	  ctxt.canonicalize_type_later(composed_type);
 	  result->set_composed_type(composed_type);
 	  break;
 	}
@@ -5517,7 +5475,7 @@  build_template_tparameter(read_context&	ctxt,
   if (result)
     {
       ctxt.key_type_decl(result, id);
-      ctxt.maybe_canonicalize_type(result, /*force_delay=*/false);
+      ctxt.canonicalize_type_later(result);
     }
 
   return result;
@@ -5607,7 +5565,7 @@  handle_type_decl(read_context&	ctxt,
 {
   type_decl_sptr decl = build_type_decl(ctxt, node, add_to_current_scope);
   if (decl && decl->get_scope())
-    ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+    ctxt.canonicalize_type_later(decl);
   return decl;
 }
 
@@ -5640,7 +5598,7 @@  handle_qualified_type_decl(read_context&	ctxt,
     build_qualified_type_decl(ctxt, node,
 			      add_to_current_scope);
   if (decl && decl->get_scope())
-    ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+    ctxt.canonicalize_type_later(decl);
   return decl;
 }
 
@@ -5657,7 +5615,7 @@  handle_pointer_type_def(read_context&	ctxt,
   pointer_type_def_sptr decl = build_pointer_type_def(ctxt, node,
 						      add_to_current_scope);
   if (decl && decl->get_scope())
-    ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+    ctxt.canonicalize_type_later(decl);
   return decl;
 }
 
@@ -5674,7 +5632,7 @@  handle_reference_type_def(read_context& ctxt,
   reference_type_def_sptr decl = build_reference_type_def(ctxt, node,
 							  add_to_current_scope);
   if (decl && decl->get_scope())
-    ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+    ctxt.canonicalize_type_later(decl);
   return decl;
 }
 
@@ -5690,7 +5648,8 @@  handle_function_type(read_context&	ctxt,
 {
   function_type_sptr type = build_function_type(ctxt, node,
 						add_to_current_scope);
-  ctxt.maybe_canonicalize_type(type, /*force_delay=*/true);
+  if (type)
+    ctxt.canonicalize_type_later(type);
   return type;
 }
 
@@ -5706,7 +5665,8 @@  handle_array_type_def(read_context&	ctxt,
 {
   array_type_def_sptr decl = build_array_type_def(ctxt, node,
 						  add_to_current_scope);
-  ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+  if (decl)
+    ctxt.canonicalize_type_later(decl);
   return decl;
 }
 
@@ -5722,7 +5682,7 @@  handle_enum_type_decl(read_context&	ctxt,
     build_enum_type_decl_if_not_suppressed(ctxt, node,
 					   add_to_current_scope);
   if (decl && decl->get_scope())
-    ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+    ctxt.canonicalize_type_later(decl);
   return decl;
 }
 
@@ -5737,7 +5697,7 @@  handle_typedef_decl(read_context&	ctxt,
   typedef_decl_sptr decl = build_typedef_decl(ctxt, node,
 					      add_to_current_scope);
   if (decl && decl->get_scope())
-    ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+    ctxt.canonicalize_type_later(decl);
   return decl;
 }
 
@@ -5788,7 +5748,7 @@  handle_class_decl(read_context& ctxt,
   class_decl_sptr decl =
     build_class_decl_if_not_suppressed(ctxt, node, add_to_current_scope);
   if (decl && decl->get_scope())
-    ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+    ctxt.canonicalize_type_later(decl);
   return decl;
 }
 
@@ -5806,7 +5766,7 @@  handle_union_decl(read_context& ctxt,
   union_decl_sptr decl =
     build_union_decl_if_not_suppressed(ctxt, node, add_to_current_scope);
   if (decl && decl->get_scope())
-    ctxt.maybe_canonicalize_type(decl, /*force_delay=*/false);
+    ctxt.canonicalize_type_later(decl);
   return decl;
 }