Re: [PATCH 4/7] abidiff: Remove member function diff blank lines.

[Dodji Seketeli <dodji@seketeli.org>]

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

[...]


>> > +struct ops {
>> > +  // TODO: type, name and size are differnent from deleted_var, but this is
>> > +  // still reported as a change rather than a deletion and an addition.
>> > +  long added_var;
>> > +  virtual long added_fn() { return 0; }
>>
>> This is by design.  If a member variable S::foo_member at a given
>> offset/ is replaced by another variable S::bar_member (foo_member and
>> bar_member have the same offset), we chose to detect that foo_member was
>> renamed into bar_member.
>>
>> So I think we can remove that TODO above.
>>
>> Would you agree?

[...]

> I think the TODO has served its purpose which was to attract attention
> (now or in the future). I'll remove it.

Thanks.  I am doing it locally here, so you don't need to send another
patch just for that.

[...]

> Here are a couple more worth thinking about:
>
> struct {
>   char padding1[16];
>   long foo;
>   char padding2[8];
> };
>
> struct {
>   char padding1[8];
>   // did foo move and get renamed?
>   long bar;
>   char padding2[16];
> };

Here is what abidiff shows for this one (I named the struct 'S'):

$ ~/git/libabigail/master/build/tools/abidiff -l test-v0.o test-v1.o 
Leaf changes summary: 1 artifact changed
Changed leaf types summary: 1 leaf type changed
Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable

'struct S at test-v0.c:1:1' changed:
  type size hasn't changed
  1 data member deletion:
    'long int S::foo', at offset 128 (in bits) at test-v0.c:3:1
  1 data member insertion:
    'long int S::bar', at offset 64 (in bits) at test-v1.c:4:1
  there are data member changes:
    type 'char[16]' of 'S::padding1' changed:
      type name changed from 'char[16]' to 'char[8]'
      array type size changed from 128 to 64
      array type subrange 1 changed length from 16 to 8
    and size changed from 128 to 64 (in bits) (by -64 bits)
    type 'char[8]' of 'S::padding2' changed:
      type name changed from 'char[8]' to 'char[16]'
      array type size changed from 64 to 128
      array type subrange 1 changed length from 8 to 16
    and offset changed from 192 to 128 (in bits) (by -64 bits), size changed from 64 to 128 (in bits) (by +64 bits)

$ 

> and
>
> struct {
>   bool flag1;
>   bool flag2;
> };
>
> struct {
>   // flag1 deleted or flag1 renamed and flag2 deleted?
>   bool flag2;
> };

And for this one, here is what abidiff shows (I renamed the struct as S
and did s/bool/char/):

$ ~/git/libabigail/master/build/tools/abidiff -l test-v0.o test-v1.o 
Leaf changes summary: 1 artifact changed
Changed leaf types summary: 1 leaf type changed
Removed/Changed/Added functions summary: 0 Removed, 0 Changed, 0 Added function
Removed/Changed/Added variables summary: 0 Removed, 0 Changed, 0 Added variable

'struct S at test-v0.c:1:1' changed:
  type size changed from 16 to 8 (in bits)
  1 data member deletion:
    'bool S::flag1', at offset 0 (in bits) at test-v0.c:2:1
  there are data member changes:
    'bool S::flag2' offset changed from 8 to 0 (in bits) (by -8 bits)

$ 

Hopefully these change reports should address your (valid!) concerns.

Thanks for looking into this!

Cheers,

-- 
		Dodji