[PATCH v2] Implement pahole-like 'ptype /o' option
Tue Dec 12 00:52:00 GMT 2017
On 12/12/2017 12:25 AM, Sergio Durigan Junior wrote:
> On Monday, December 11 2017, Pedro Alves wrote:
>> IMO, the right way to go about this is to decide on a format,
>> document it in the wiki and then we can point everyone at it with a URL.
>> (ISTR that GCC's coding conventions documents yet another
>> way to break the line, and in that case, not even GCC follows it.)
>> IMO, the format you've chosen isn't ideal because it requires
>> manual right-alignment for every line, while breaking before the
>> parens makes emacs's TAB automatically align the following lines.
>> The latter also gives a lot more space for the params again; it's
>> sort of a "well, I have to do something, so might as well start
>> way back from the left again).
> No, it's not ideal at all. But at least on my Emacs, the other format
> has the downside of being always realigned to the column zero when I
> press TAB on it. But again, I'm not saying this is Emacs fault nor your
> nor Simon's fault; this is just something I noticed.
Seems easier to fix (just two spaces, and only on one line) than
the other approach, which depends a varying number of
space/delete strokes. Maybe there's a way to configure emacs
not to do that, even?
>>>>>> But I don't mind it, it just stuck out as a little inconsistency.
>>>>> I don't see the inconsistency.
>>>>> If a field is inside a struct, it has its offset *and* size printed. No
>>>>> matter if the field is an int, another struct, or an union.
>>>>> If a field is inside an union, it has only its size printed.
>>>>> In the case above, it makes sense to have the offsets printed for the
>>>>> fields inside the two structs (inside the union), because there might be
>>>>> holes to report (well, one can argue that it doesn't matter if there are
>>>>> holes or not in this case, because if the other struct is bigger then
>>>>> the union size will stay the same). However, it doesn't make sense to
>>>>> print the offsets for the two structs themselves, because they are
>>>>> members of the union.
>>>>> I hope it makes more sense now.
>>>> But why do we need the special case? Does it help anything?
>>>> So far, it seems it only added confusion.
>>> What do you mean by "special case"?
>> Special case:
>> "If a field is inside a union, it has only its size printed; otherwise
>> print the offset and size."
>> No special case:
>> "Always print the offset and size."
> I don't like the expression "special case" because it diminishes the
> difference that exist between structs and unions.
I don't follow, but OK...
> It is not like I went
> out of my way to treat this difference and made the code complex; it is
> also not like the output is extremely complex with it.
The point isn't about the implementation complexity, it's about
user expectations. Simon was seemingly surprised by "an inconsistency"
(i.e., a case is not consistent with the others; i.e., there's
special/different case), so I think it's valid to discuss a bit and maybe
double check the rationale and see if we can save users from being confused too.
If after chatting a bit we come to the conclusion skipping the offsets
makes sense, than that's fine. No harm done.
>>> I don't consider this a
>>> special case; I consider it to be the natural thing to do, because
>>> offsets don't make much sense in unions.
>> Of course they do. You can do 'offsetof(foo_union, foo_field)' just
>> fine, for example. Saying that the offsets happen to be the same
>> is not the same as saying that the offsets don't exist.
> I don't remember saying offsets don't exist in unions. What I said is
> that in this specific case they don't matter/make much sense to be
Guess we're discussing semantics, which is kind of pointless...
"Don't make sense" to me is like talking about what's the
"weight of a mile", which is truly meaningless. Stating that
all union members live at offset 0 is not meaningless, because
that's exactly how you define a union!
>> So from that angle, I see value in not printing the offsets
>> of union members.
> Since it's still not clear whether the offsets should be printed or not
> in this case, and I am not a global maintainer, I adjusted the code to
> print them and will post the patch as a reply to the v4 e-mail. This
> way you can decide which version is best.
Fun, just when I agreed with not printing the offsets... :-P :-)
More information about the Gdb-patches