Re: [PATCH] abidiff: improve treatment of array types in leaf changes mode

[Dodji Seketeli <dodji@seketeli.org>]

Hello Giuliano,

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

[...]

> diff --git a/src/abg-ir.cc b/src/abg-ir.cc
> index c6f7c13e..b0db9c39 100644
> --- a/src/abg-ir.cc
> +++ b/src/abg-ir.cc
> @@ -23606,7 +23606,7 @@ types_have_similar_structure(const type_base* first,
>  	  || ty1->get_dimension_count() != ty2->get_dimension_count()
>  	  || !types_have_similar_structure(ty1->get_element_type(),
>  					   ty2->get_element_type(),
> -					   indirect_type))
> +					   true))
>  	return false;
>  
>        return true;

I think this change is correct, thanks for spotting that.

However ...

[...]

> If an array's element type doesn't change name but has some other
> (local) change, then the change should not also be considered local to
> the array type.

I find this comment confusing, even if I think I see what you mean.

A change being local or not, is a concept that is not at the same
logical level as the concept of "type similarity" defined in the comment
of the function types_have_similar_structure.  I would say that the type
similarity concept is at a lower logical level.  So seeing this comment
that applies to a higher level concept for a change made to something

But I agree that the comments of types_have_similar_structure are
confusing as well.

So I am proposing two patches following this message, for this issue.
One patch amends the the comments for types_have_similar_structure, and
the second one is your patch, with amended comments.

And we can discuss from there as you like.

[...]

Cheers,

-- 
		Dodji