Re: [Bug default/27569] abidiff misses a function parameter addition

[Dodji Seketeli <dodji@seketeli.org>]

Giuliano Procida via Libabigail <libabigail@sourceware.org> a écrit:

> I had a quick look. It seems to do the trick.

Thanks.

> I don't think you need to include new test files as at least one existing
> test is affected by the change.

Stricto sensu, you are right.  There are several other tests affected by
the change.

The reason why I am keeping this one is that it's context is a little
different from the others.

If anything, it's doing the analysis from
abixml files.

Also, looking at things a little deeper the function type impacted by
the change is also used in other data structures in the ABI graph and
that has intersting interactions in terms of the various categorizations
passes involved.  I think it is interesting to see the change being
detected and reported in that context.

> A couple of questions:
>
> Can any other function level changes be missed? I'm guessing not, from
> looking at the XML attributes on a function-decl.
>
> In the bug, I wrote:
>
> I also think the only reason problems have not been noticed before is that
> is_filtered_out contains the suspicious-looking:
>
>   if (category == NO_CHANGE_CATEGORY)
>     return false;

The NO_CHANGE_CATEGORY lumps together all changes that haven't yet been
categorized.  So what this line says is that if we are looking at a
change that is not (yet) categorized, then do not filter it out.

So it's not suspicious.

>
> If I remove this line and re-run tests, some other diffs disappear, such as:
>
> ---
> /usr/local/google/home/gprocida/android/libabigail/build/../tests/data/test-diff-suppr/test10-changed-parm-c-report-0.txt
>   2021-02-03 10:29:34.846116830 +0000
> +++
> /usr/local/google/home/gprocida/android/libabigail/build/tests/output/test-diff-suppr/test10-changed-parm-c-report-0.txt
>    2021-03-25 16:28:56.472979039 +0000
> @@ -1,16 +1,3 @@
> -Functions changes summary: 0 Removed, 1 Changed, 0 Added function
> +Functions changes summary: 0 Removed, 0 Changed (1 filtered out), 0 Added
> function
>  Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
>
> -1 function with some indirect sub-type change:
> -
> -  [C] 'function int foo(int, int)' has some indirect sub-type changes:
> -    return type changed:
> -      type name changed from 'int' to 'float'
> -      type size hasn't changed
> -    parameter 1 of type 'int' changed:
> -      type name changed from 'int' to 'float'
> -      type size hasn't changed
> -    parameter 2 of type 'int' changed:
> -      type name changed from 'int' to 'float'
> -      type size hasn't changed
> -
>
> Does this mean you are also missing a category for "has changed to a
> different type"? Or is that somehow the residual meaning
> of NO_CHANGE_CATEGORY?

From what I said above, I think it's more of the latter.

Cheers,

-- 
		Dodji