[PATCH 0/3] Type equality refactor and instrumentation

[Dodji Seketeli]

Hello Giuliano,

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

> Hi Dodji.
>
> This series refactors various operator== methods making their common
> functionality evident.
>
> The first patch is just a prelude to make the second smaller.
>
> The second patch does the refactoring. I'm not attached to the name
> 'equality_helper'.

Thank you for working on this.

I have looked into these first two patches and they look good to me in general.
I'll post their individual review as usual, shortly.

> The third patch is not intended for direct inclusion in libabigail but
> builds on the refactoring to investigate how equality and canonical
> types work in practice. It identifies some potential discrepancies,
> but they may be entirely expected.

In it's current form, I'd rather hold the inclusion of this one for now.
It's super intrusive, clutters the code quite a bit and I am not really
sure about its practical use at the moment.

> In general, it can be risky to define operator== in a way where
> reflexivity, symmetry and transitivity do not obviously hold or can be
> sensitive to small code changes or in way where equality, say for
> class_decl_sptr, can be affected by something like canonicalisation.
> More instrumentation could be added to check behaviour.

The comparison code is tricky.  The number one reason for this is that
it has to be fast.  So yes, it's risky.

One category of tests that are "simple" to perform and in practise are
quite powerful to detect and debug potential comparison issues are
"identity tests".  That is, comparing a binary against itself and
requiring that the result be the empty set.  This is why the option
"--abidiff" was added to abidw.

So I tend to favor schemes that keep the code the less cluttered and the
most "debuggable" as possible, as opposed to adding a lot of
instrumentation.  I guess some balance has to found there.

Cheers,

-- 
		Dodji