[3/3] Simplify peel_typedef_pointer_or_reference_type

  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>
---
 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(-)
  

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
>
  

Giuliano Procida <gprocida@google.com> a écrit:

[...]

>
> 	* 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>

Applied to master, thanks!

[...]

Cheers,