Re: [PATCH v2] Implement pahole-like 'ptype /o' option

[Simon Marchi <simon.marchi@ericsson.com>]

>>> +/* Use 'print_spaces_filtered', but take into consideration the
>>> +   type_print_options FLAGS in order to determine how many whitespaces
>>> +   will be printed.  */
>>> +
>>> +static void
>>> +print_spaces_filtered_with_print_options (int level, struct ui_file *stream,
>>> +					const struct type_print_options *flags)
>>
>> Missing spaces here.
> 
> This is actually on purpose.  If I indent the line, it will have more
> than 80 chars.  I believe this is a well known method for avoiding this
> problem...?

I am not aware of that.  In this case I would put the parameter list on the next,
I'm not sure if it's 100% GNU-style approved, but nobody complained when I did it
so far :)

static void
print_spaces_filtered_with_print_options
  (int level, struct ui_file *stream, const struct type_print_options *flags);

It helps with long function names.  In this case, I would probably just drop the
"struct" to save a few chars, because C++.

>>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile \
>>> +	  { debug c++ optimize=-O0 }] } {
>>
>> optimize=-O0 seems unnecessary to me, I've never seen it specified explicitly in a test.
> 
> There are very few tests that use it (2, currently).  I understand it's
> not very common to explicitly specify -O0, but I put it there because I
> decided to be on the safe side.  If the compiler performs any
> optimization at all, it could mess with the layout of the structs.

If GCC decided to optimize by default, so many things would break in the GDB testsuite.
We would then probably make gdb_compile add -O0, so that we wouldn't need to do it in
all tests.  The point is that IMO, tests should expect no optimization by default.

>> I also noticed that the offset is not shown in front of the struct-in-union,
>> as show above, but it is in the case of struct-in-struct:
>>
>>   /* offset    |  size */
>>   type = struct my_struct_3 {
>>   /*    0      |     8 */    struct my_struct_1 {
>>   /*    0      |     4 */        int a;
>>   /*    4      |     4 */        int b;
>>                              } /* total size:    8 bytes */ s1;
>>   /*    8      |     8 */    struct my_struct_2 {
>>   /*    8      |     4 */        int c;
>>   /*   12      |     4 */        int d;
>>                              } /* total size:    8 bytes */ s2;
>>   } /* total size:   16 bytes */
>>
>> Is this difference on purpose?
> 
> Yes; offsets are not shown for fields inside unions (not only structs,
> but all types of fields), because it doesn't make much sense: they'd be
> 0 every time.  This is also inspired from pahole's output.

Not if that union is itself in a struct.  For example with this:

struct hello
{
        int i;
        union {
                struct {
                        int x, y;
                } a;
                struct {
                        int x, y;
                } b;
        };
};

(gdb) ptype /o struct hello
/* offset    |  size */
type = struct hello {
/*    0      |     4 */    int i;
/*    4      |     8 */    union {
/*                 8 */        struct {
/*    4      |     4 */            int x;
/*    8      |     4 */            int y;
                               } /* total size:    8 bytes */ a;
/*                 8 */        struct {
/*    4      |     4 */            int x;
/*    8      |     4 */            int y;
                               } /* total size:    8 bytes */ b;
                           } /* total size:    8 bytes */;
} /* total size:   12 bytes */


But I don't mind it, it just stuck out as a little inconsistency.

I'll look at v3 now.

Simon