Re: [PATCH 3/5] XML writer: track emitted types by bare pointer

[Dodji Seketeli <dodji@seketeli.org>]

Hello,

Matthias Maennich <maennich@google.com> a écrit:

[...]

> This is a performance and safety improvement made possible by the
> previous changes which ensure that the same pointers are inserted and
> looked up.
>
> This essentially removes the now unnecessary deep comparison.

[...]

> +++ b/src/abg-writer.cc
> @@ -123,14 +123,10 @@ typedef unordered_map<type_base*,
>  		      abigail::diff_utils::deep_ptr_eq_functor> type_ptr_map;
>  
>  // A convenience typedef for a set of type_base*.
> -typedef unordered_set<const type_base*, type_hasher,
> -		      abigail::diff_utils::deep_ptr_eq_functor>
> -type_ptr_set_type;
> +typedef std::unordered_set<const type_base*> type_ptr_set_type;
>  
>  /// A convenience typedef for a set of function type*.
> -typedef unordered_set<function_type*, type_hasher,
> -		      abigail::diff_utils::deep_ptr_eq_functor>
> -fn_type_ptr_set_type;
> +typedef std::unordered_set<function_type*> fn_type_ptr_set_type;

The problem I see with doing this is that it's possible that two
declaration-only classes, that are equivalent but that have different
pointer values get into these sets.

In that case, they would be considered different even though they are
not.

So maybe it would be better have an equality operator that uses
is_non_canonicalized_type() to detect those rare cases and use
structural comparison in those cases?

What do you think?

-- 
		Dodji