[PATCH 3/3] Simplify peel_typedef_pointer_or_reference_type

Matthias Maennich maennich@google.com
Fri Jul 10 16:17:23 GMT 2020


On Thu, Jul 09, 2020 at 02:14:29PM +0100, Giuliano Procida wrote:
>The second argument to this function controls whether CV qualifiers
>should be stripped as well as the elements mentioned in the function
>name. While it defaults to true, it is now always passed in as false.
>
>The function contains incomplete code for peeling array types. This is
>not needed or indeed wanted by its current callers.
>
>This commits removes the peel_qual_type argument and removes the
>associated behaviour. It removes the array peeling code.
>
>There are no functional changes apart from no longer performing early
>canonicalisation of certain array types.
>
>I looked at the history of this function to see where the behaviours
>originated.
>
>bd161caa Make type_has_non_canonicalized_subtype() tighter
>
>The function is added to help make more types, self-referential ones,
>candidates for early canonicalisation.
>
>5822798d Bug 18894 - Fix representation of enumerators in abixml format
>
>This undid the previous change but the function remained.
>
>c20c8c79 Fix infinite loop in peel_typedef_pointer_or_reference_type
>
>As it says, fixing the overload that is currently in use.
>
>6e36a438 Late canonicalize all types that reference classes when reading DWARF
>
>This reintroduced the use of the function to control canonicalisation
>by the detection of class types. It also added array peeling to the
>function, but in a broken fashion as it would only work for certain
>combinations of pointers, references or typedefs referring to arrays.
>
>e9bdb488 Bug 19025 - abixml writer forgets to emit some member types
>
>This added a use of the function in a map key comparison function.
>
>8cc382c8 Fix emitting of referenced type in abixml writer
>
>This undid the previous change.
>
>1bee40c0 Do not forget to peel qualified type off when peeling types
>
>This made the function remove CV qualifiers unconditionally.
>
>e73901a5 Do not mark "distinct" diff nodes as being redundant
>
>This made behaviour to remove CV qualifiers optional and newly added
>is_mostly_distinct_diff disabled it.
>
>5d6af8d5 Delay canonicalization for array and qualified types
>
>This change switches maybe_canonicalize_type to not request CV
>qualifer peeling from peel_typedef_pointer_or_reference_type.
>
>It partially resolves the array type issue as they are separately
>checked for. Presumably they shouldn't be peeled, but still are under
>some circumstances.
>
>The tests here could be subject to further refinement. Many types have
>delayed canonicalisation already.
>
>9cf76b11 abg-ir.cc: Improve types_have_similar_structure.
>
>This change replaced the use of the function with a more delicate
>matched peeling process for pointer and reference types plus
>peel_qualified_or_typedef_type. It obsoleted the behaviour where CV
>qualifiers were stripped.
>
>	* include/abg-fwd.h (peel_qualified_or_typedef_type): Remove
>	second argument in declarations of both overloads.
>	* src/abg-comp-filter.cc (is_mostly_distinct_diff): Remove
>	second argument to peel_qualified_or_typedef_type.
>	* src/abg-dwarf-reader.cc (maybe_canonicalize_type): Likewise.
>	* src/abg-ir.cc (peel_qualified_or_typedef_type): In both
>	overloads, remove second argument peel_qual_type, simplify
>	code with the assumption it was always false and remove
>	incomplete array type peeling logic. In type_base_sptr
>	overload, remove stray space.
>
>Signed-off-by: Giuliano Procida <gprocida@google.com>

Nice archaeology work :-)

Reviewed-by: Matthias Maennich <maennich@google.com>

Cheers,
Matthias

>---
> include/abg-fwd.h       |  6 ++----
> src/abg-comp-filter.cc  |  6 ++----
> src/abg-dwarf-reader.cc |  6 ++----
> src/abg-ir.cc           | 40 +++++++++-------------------------------
> 4 files changed, 15 insertions(+), 43 deletions(-)
>
>diff --git a/include/abg-fwd.h b/include/abg-fwd.h
>index f23f4a46..14b95a96 100644
>--- a/include/abg-fwd.h
>+++ b/include/abg-fwd.h
>@@ -819,12 +819,10 @@ type_base*
> peel_qualified_or_typedef_type(const type_base* type);
>
> type_base_sptr
>-peel_typedef_pointer_or_reference_type(const type_base_sptr,
>-				       bool peel_qualified_type = true);
>+peel_typedef_pointer_or_reference_type(const type_base_sptr);
>
> type_base*
>-peel_typedef_pointer_or_reference_type(const type_base* type,
>-				       bool peel_qualified_type = true);
>+peel_typedef_pointer_or_reference_type(const type_base* type);
>
> type_base*
> peel_pointer_or_reference_type(const type_base *type,
>diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc
>index 702d223f..a9bf2c70 100644
>--- a/src/abg-comp-filter.cc
>+++ b/src/abg-comp-filter.cc
>@@ -1067,10 +1067,8 @@ is_mostly_distinct_diff(const diff *d)
>   type_base_sptr first = is_type(td->first_subject());
>   type_base_sptr second = is_type(td->second_subject());
>
>-  first = peel_typedef_pointer_or_reference_type(first,
>-						 /*peel_qualified_type=*/false);
>-  second = peel_typedef_pointer_or_reference_type(second,
>-						  /*peel_qual_type=*/false);
>+  first = peel_typedef_pointer_or_reference_type(first);
>+  second = peel_typedef_pointer_or_reference_type(second);
>   ABG_ASSERT(first && second);
>
>   return distinct_diff::entities_are_of_distinct_kinds(first, second);
>diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc
>index ba4e750f..054ac91b 100644
>--- a/src/abg-dwarf-reader.cc
>+++ b/src/abg-dwarf-reader.cc
>@@ -15740,8 +15740,7 @@ maybe_canonicalize_type(const Dwarf_Die *die, read_context& ctxt)
>   if (!t)
>     return;
>
>-  type_base_sptr peeled_type =
>-    peel_typedef_pointer_or_reference_type(t, /*peel_qual_types=*/false);
>+  type_base_sptr peeled_type = peel_typedef_pointer_or_reference_type(t);
>   if (is_class_type(peeled_type)
>       || is_union_type(peeled_type)
>       || is_function_type(peeled_type)
>@@ -15794,8 +15793,7 @@ maybe_canonicalize_type(const type_base_sptr& t,
>   if (!t)
>     return;
>
>-  type_base_sptr peeled_type =
>-    peel_typedef_pointer_or_reference_type(t, /*peel_qual_types=*/false);
>+  type_base_sptr peeled_type = peel_typedef_pointer_or_reference_type(t);
>   if (is_class_type(peeled_type)
>       || is_union_type(peeled_type)
>       || is_function_type(peeled_type)
>diff --git a/src/abg-ir.cc b/src/abg-ir.cc
>index a434ec69..5ad60dfa 100644
>--- a/src/abg-ir.cc
>+++ b/src/abg-ir.cc
>@@ -5707,23 +5707,19 @@ peel_qualified_or_typedef_type(const type_base* type)
> }
>
> /// Return the leaf underlying or pointed-to type node of a @ref
>-/// typedef_decl, @ref pointer_type_def, @ref reference_type_def or
>-/// @ref qualified_type_def node.
>+/// typedef_decl, @ref pointer_type_def or @ref reference_type_def
>+/// node.
> ///
> /// @param type the type to peel.
> ///
>-/// @param peel_qualified_type if true, also peel qualified types.
>-///
> /// @return the leaf underlying or pointed-to type node of @p type.
> type_base_sptr
>-peel_typedef_pointer_or_reference_type(const type_base_sptr type,
>-				       bool peel_qual_type)
>+peel_typedef_pointer_or_reference_type(const type_base_sptr type)
> {
>-  type_base_sptr typ  = type;
>+  type_base_sptr typ = type;
>   while (is_typedef(typ)
> 	 || is_pointer_type(typ)
>-	 || is_reference_type(typ)
>-	 || (peel_qual_type && is_qualified_type(typ)))
>+	 || is_reference_type(typ))
>     {
>       if (typedef_decl_sptr t = is_typedef(typ))
> 	typ = peel_typedef_type(t);
>@@ -5733,35 +5729,24 @@ peel_typedef_pointer_or_reference_type(const type_base_sptr type,
>
>       if (reference_type_def_sptr t = is_reference_type(typ))
> 	typ = peel_reference_type(t);
>-
>-      if (array_type_def_sptr t = is_array_type(typ))
>-	typ = peel_array_type(t);
>-
>-      if (peel_qual_type)
>-	if (qualified_type_def_sptr t = is_qualified_type(typ))
>-	  typ = peel_qualified_type(t);
>     }
>
>   return typ;
> }
>
> /// Return the leaf underlying or pointed-to type node of a @ref
>-/// typedef_decl, @ref pointer_type_def, @ref reference_type_def or
>-/// @ref qualified_type_def type node.
>+/// typedef_decl, @ref pointer_type_def or @ref reference_type_def
>+/// node.
> ///
> /// @param type the type to peel.
> ///
>-/// @param peel_qualified_type if true, also peel qualified types.
>-///
> /// @return the leaf underlying or pointed-to type node of @p type.
> type_base*
>-peel_typedef_pointer_or_reference_type(const type_base* type,
>-				       bool peel_qual_type)
>+peel_typedef_pointer_or_reference_type(const type_base* type)
> {
>   while (is_typedef(type)
> 	 || is_pointer_type(type)
>-	 || is_reference_type(type)
>-	 || (peel_qual_type && is_qualified_type(type)))
>+	 || is_reference_type(type))
>     {
>       if (const typedef_decl* t = is_typedef(type))
> 	type = peel_typedef_type(t);
>@@ -5771,13 +5756,6 @@ peel_typedef_pointer_or_reference_type(const type_base* type,
>
>       if (const reference_type_def* t = is_reference_type(type))
> 	type = peel_reference_type(t);
>-
>-      if (const array_type_def* t = is_array_type(type))
>-	type = peel_array_type(t);
>-
>-      if (peel_qual_type)
>-	if (const qualified_type_def* t = is_qualified_type(type))
>-	  type = peel_qualified_type(t);
>     }
>
>   return const_cast<type_base*>(type);
>-- 
>2.27.0.383.g050319c2ae-goog
>


More information about the Libabigail mailing list