[1/7] Simplify type equality infinite recursion logic

Message ID 20200715081631.2076978-1-gprocida@google.com
State Dropped, archived
Headers
Series [1/7] Simplify type equality infinite recursion logic |

Commit Message

Giuliano Procida July 15, 2020, 8:16 a.m. UTC
  Function types don't introduce new names so cannot cause self
references and so there is no need to track currently being compared
function types.

Typedefs do introduce new names but cannot refer directly to
themselves or be forward-declared. Enums introduce new names but
cannot refer to anything other than integral types. Therefore each
self reference must be via a class, struct or union type.

This commit removes the extra logic for function types.

Note that this short-cutting could actually be done at every level of
type comparison but it's unknown whether this would buy any
performance advantage.

	* src/abg-ir.cc (struct environment::priv): Drop member
	fn_types_being_compared_.
	(mark_as_being_compared): Drop function_type overload.
	(unmark_as_being_compared): Likewise.
	(comparison_started): Likewise.
	(equals): In the function_type overload, inline and remove the
	RETURN macro, bearing in mind that its argument was almost
	always false and the unmark_as_being_compared method is gone.
	(types_are_being_compared): Remove the code relating to
	function types being compared.

Signed-off-by: Giuliano Procida <gprocida@google.com>
---
 src/abg-ir.cc | 80 +++++++--------------------------------------------
 1 file changed, 11 insertions(+), 69 deletions(-)
  

Comments

Giuliano Procida July 15, 2020, 8:17 a.m. UTC | #1
This was sent prematurely, please ignore for now.

Regards,
Giuliano.

On Wed, 15 Jul 2020 at 09:16, Giuliano Procida <gprocida@google.com> wrote:
>
> Function types don't introduce new names so cannot cause self
> references and so there is no need to track currently being compared
> function types.
>
> Typedefs do introduce new names but cannot refer directly to
> themselves or be forward-declared. Enums introduce new names but
> cannot refer to anything other than integral types. Therefore each
> self reference must be via a class, struct or union type.
>
> This commit removes the extra logic for function types.
>
> Note that this short-cutting could actually be done at every level of
> type comparison but it's unknown whether this would buy any
> performance advantage.
>
>         * src/abg-ir.cc (struct environment::priv): Drop member
>         fn_types_being_compared_.
>         (mark_as_being_compared): Drop function_type overload.
>         (unmark_as_being_compared): Likewise.
>         (comparison_started): Likewise.
>         (equals): In the function_type overload, inline and remove the
>         RETURN macro, bearing in mind that its argument was almost
>         always false and the unmark_as_being_compared method is gone.
>         (types_are_being_compared): Remove the code relating to
>         function types being compared.
>
> Signed-off-by: Giuliano Procida <gprocida@google.com>
> ---
>  src/abg-ir.cc | 80 +++++++--------------------------------------------
>  1 file changed, 11 insertions(+), 69 deletions(-)
>
> diff --git a/src/abg-ir.cc b/src/abg-ir.cc
> index 6040e1d7..5d605dd7 100644
> --- a/src/abg-ir.cc
> +++ b/src/abg-ir.cc
> @@ -2636,7 +2636,6 @@ struct environment::priv
>    type_base_sptr                void_type_;
>    type_base_sptr                variadic_marker_type_;
>    unordered_set<const class_or_union*> classes_being_compared_;
> -  unordered_set<const function_type*>  fn_types_being_compared_;
>    vector<type_base_sptr>        extra_live_types_;
>    interned_string_pool          string_pool_;
>    bool                          canonicalization_is_done_;
> @@ -16448,44 +16447,7 @@ struct function_type::priv
>    priv(type_base_sptr return_type)
>      : return_type_(return_type)
>    {}
> -
> -  /// Mark a given @ref function_type as being compared.
> -  ///
> -  /// @param type the @ref function_type to mark as being compared.
> -  void
> -  mark_as_being_compared(const function_type& type) const
> -  {
> -    const environment* env = type.get_environment();
> -    ABG_ASSERT(env);
> -    env->priv_->fn_types_being_compared_.insert(&type);
> -  }
> -
> -  /// If a given @ref function_type was marked as being compared, this
> -  /// function unmarks it.
> -  ///
> -  /// @param type the @ref function_type to mark as *NOT* being
> -  /// compared.
> -  void
> -  unmark_as_being_compared(const function_type& type) const
> -  {
> -    const environment* env = type.get_environment();
> -    ABG_ASSERT(env);
> -    env->priv_->fn_types_being_compared_.erase(&type);
> -  }
> -
> -  /// Tests if a @ref function_type is currently being compared.
> -  ///
> -  /// @param type the function type to take into account.
> -  ///
> -  /// @return true if @p type is being compared.
> -  bool
> -  comparison_started(const function_type& type) const
> -  {
> -    const environment* env = type.get_environment();
> -    ABG_ASSERT(env);
> -    return env->priv_->fn_types_being_compared_.count(&type);
> -  }
> -};// end struc function_type::priv
> +};// end struct function_type::priv
>
>  /// This function is automatically invoked whenever an instance of
>  /// this type is canonicalized.
> @@ -16718,22 +16680,6 @@ equals(const function_type& lhs,
>         const function_type& rhs,
>         change_kind* k)
>  {
> -#define RETURN(value)                          \
> -  do {                                         \
> -    lhs.priv_->unmark_as_being_compared(lhs);  \
> -    lhs.priv_->unmark_as_being_compared(rhs);  \
> -    if (value == true)                         \
> -      maybe_propagate_canonical_type(lhs, rhs); \
> -    return value;                              \
> -  } while(0)
> -
> -  if (lhs.priv_->comparison_started(lhs)
> -      || lhs.priv_->comparison_started(rhs))
> -    return true;
> -
> -  lhs.priv_->mark_as_being_compared(lhs);
> -  lhs.priv_->mark_as_being_compared(rhs);
> -
>    bool result = true;
>
>    if (!lhs.type_base::operator==(rhs))
> @@ -16742,7 +16688,7 @@ equals(const function_type& lhs,
>        if (k)
>         *k |= LOCAL_TYPE_CHANGE_KIND;
>        else
> -       RETURN(result);
> +       return result;
>      }
>
>    class_or_union* lhs_class = 0, *rhs_class = 0;
> @@ -16760,7 +16706,7 @@ equals(const function_type& lhs,
>        if (k)
>         *k |= LOCAL_TYPE_CHANGE_KIND;
>        else
> -       RETURN(result);
> +       return result;
>      }
>    else if (lhs_class
>            && (lhs_class->get_qualified_name()
> @@ -16770,7 +16716,7 @@ equals(const function_type& lhs,
>        if (k)
>         *k |= LOCAL_TYPE_CHANGE_KIND;
>        else
> -       RETURN(result);
> +       return result;
>      }
>
>    // Then compare the return type; Beware if it's t's a class type
> @@ -16808,7 +16754,7 @@ equals(const function_type& lhs,
>                 *k |= SUBTYPE_CHANGE_KIND;
>             }
>           else
> -           RETURN(result);
> +           return result;
>         }
>      }
>    else
> @@ -16818,7 +16764,7 @@ equals(const function_type& lhs,
>         if (k)
>           *k |= SUBTYPE_CHANGE_KIND;
>         else
> -         RETURN(result);
> +         return result;
>        }
>
>    class_decl* lcl = 0, * rcl = 0;
> @@ -16851,7 +16797,7 @@ equals(const function_type& lhs,
>                 *k |= SUBTYPE_CHANGE_KIND;
>             }
>           else
> -           RETURN(result);
> +           return result;
>         }
>      }
>
> @@ -16862,11 +16808,12 @@ equals(const function_type& lhs,
>        if (k)
>         *k |= LOCAL_NON_TYPE_CHANGE_KIND;
>        else
> -       RETURN(result);
> +       return result;
>      }
>
> -  RETURN(result);
> -#undef RETURN
> +  if (result)
> +    maybe_propagate_canonical_type(lhs, rhs);
> +  return result;
>  }
>
>  /// Get the parameter of the function.
> @@ -19594,11 +19541,6 @@ types_are_being_compared(const type_base& lhs_type,
>        return (l_cou->priv_->comparison_started(*l_cou)
>               || l_cou->priv_->comparison_started(*r_cou));
>
> -  if (function_type *l_fn_type = is_function_type(l))
> -    if (function_type *r_fn_type = is_function_type(r))
> -      return (l_fn_type->priv_->comparison_started(*l_fn_type)
> -             || l_fn_type->priv_->comparison_started(*r_fn_type));
> -
>    return false;
>  }
>
> --
> 2.27.0.389.gc38d7665816-goog
>
  

Patch

diff --git a/src/abg-ir.cc b/src/abg-ir.cc
index 6040e1d7..5d605dd7 100644
--- a/src/abg-ir.cc
+++ b/src/abg-ir.cc
@@ -2636,7 +2636,6 @@  struct environment::priv
   type_base_sptr		 void_type_;
   type_base_sptr		 variadic_marker_type_;
   unordered_set<const class_or_union*>	classes_being_compared_;
-  unordered_set<const function_type*>	fn_types_being_compared_;
   vector<type_base_sptr>	 extra_live_types_;
   interned_string_pool		 string_pool_;
   bool				 canonicalization_is_done_;
@@ -16448,44 +16447,7 @@  struct function_type::priv
   priv(type_base_sptr return_type)
     : return_type_(return_type)
   {}
-
-  /// Mark a given @ref function_type as being compared.
-  ///
-  /// @param type the @ref function_type to mark as being compared.
-  void
-  mark_as_being_compared(const function_type& type) const
-  {
-    const environment* env = type.get_environment();
-    ABG_ASSERT(env);
-    env->priv_->fn_types_being_compared_.insert(&type);
-  }
-
-  /// If a given @ref function_type was marked as being compared, this
-  /// function unmarks it.
-  ///
-  /// @param type the @ref function_type to mark as *NOT* being
-  /// compared.
-  void
-  unmark_as_being_compared(const function_type& type) const
-  {
-    const environment* env = type.get_environment();
-    ABG_ASSERT(env);
-    env->priv_->fn_types_being_compared_.erase(&type);
-  }
-
-  /// Tests if a @ref function_type is currently being compared.
-  ///
-  /// @param type the function type to take into account.
-  ///
-  /// @return true if @p type is being compared.
-  bool
-  comparison_started(const function_type& type) const
-  {
-    const environment* env = type.get_environment();
-    ABG_ASSERT(env);
-    return env->priv_->fn_types_being_compared_.count(&type);
-  }
-};// end struc function_type::priv
+};// end struct function_type::priv
 
 /// This function is automatically invoked whenever an instance of
 /// this type is canonicalized.
@@ -16718,22 +16680,6 @@  equals(const function_type& lhs,
        const function_type& rhs,
        change_kind* k)
 {
-#define RETURN(value)				\
-  do {						\
-    lhs.priv_->unmark_as_being_compared(lhs);	\
-    lhs.priv_->unmark_as_being_compared(rhs);	\
-    if (value == true)				\
-      maybe_propagate_canonical_type(lhs, rhs); \
-    return value;				\
-  } while(0)
-
-  if (lhs.priv_->comparison_started(lhs)
-      || lhs.priv_->comparison_started(rhs))
-    return true;
-
-  lhs.priv_->mark_as_being_compared(lhs);
-  lhs.priv_->mark_as_being_compared(rhs);
-
   bool result = true;
 
   if (!lhs.type_base::operator==(rhs))
@@ -16742,7 +16688,7 @@  equals(const function_type& lhs,
       if (k)
 	*k |= LOCAL_TYPE_CHANGE_KIND;
       else
-	RETURN(result);
+	return result;
     }
 
   class_or_union* lhs_class = 0, *rhs_class = 0;
@@ -16760,7 +16706,7 @@  equals(const function_type& lhs,
       if (k)
 	*k |= LOCAL_TYPE_CHANGE_KIND;
       else
-	RETURN(result);
+	return result;
     }
   else if (lhs_class
 	   && (lhs_class->get_qualified_name()
@@ -16770,7 +16716,7 @@  equals(const function_type& lhs,
       if (k)
 	*k |= LOCAL_TYPE_CHANGE_KIND;
       else
-	RETURN(result);
+	return result;
     }
 
   // Then compare the return type; Beware if it's t's a class type
@@ -16808,7 +16754,7 @@  equals(const function_type& lhs,
 		*k |= SUBTYPE_CHANGE_KIND;
 	    }
 	  else
-	    RETURN(result);
+	    return result;
 	}
     }
   else
@@ -16818,7 +16764,7 @@  equals(const function_type& lhs,
 	if (k)
 	  *k |= SUBTYPE_CHANGE_KIND;
 	else
-	  RETURN(result);
+	  return result;
       }
 
   class_decl* lcl = 0, * rcl = 0;
@@ -16851,7 +16797,7 @@  equals(const function_type& lhs,
 		*k |= SUBTYPE_CHANGE_KIND;
 	    }
 	  else
-	    RETURN(result);
+	    return result;
 	}
     }
 
@@ -16862,11 +16808,12 @@  equals(const function_type& lhs,
       if (k)
 	*k |= LOCAL_NON_TYPE_CHANGE_KIND;
       else
-	RETURN(result);
+	return result;
     }
 
-  RETURN(result);
-#undef RETURN
+  if (result)
+    maybe_propagate_canonical_type(lhs, rhs);
+  return result;
 }
 
 /// Get the parameter of the function.
@@ -19594,11 +19541,6 @@  types_are_being_compared(const type_base& lhs_type,
       return (l_cou->priv_->comparison_started(*l_cou)
 	      || l_cou->priv_->comparison_started(*r_cou));
 
-  if (function_type *l_fn_type = is_function_type(l))
-    if (function_type *r_fn_type = is_function_type(r))
-      return (l_fn_type->priv_->comparison_started(*l_fn_type)
-	      || l_fn_type->priv_->comparison_started(*r_fn_type));
-
   return false;
 }