This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Remove CHECK_TYPEDEF, use check_typedef instead
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Doug Evans <dje at google dot com>, Pedro Alves <palves at redhat dot com>
- Cc: gdb-patches <gdb-patches at sourceware dot org>
- Date: Mon, 13 Jul 2015 13:18:21 -0400
- Subject: Re: [PATCH] Remove CHECK_TYPEDEF, use check_typedef instead
- Authentication-results: sourceware.org; auth=none
- References: <1436213157-21480-1-git-send-email-simon dot marchi at ericsson dot com> <559BFB12 dot 6050606 at redhat dot com> <CADPb22SjePg2g8kn+u7tGNN2u8xxTStd-cvtQxd6BaPJ6MjWYw at mail dot gmail dot com>
On 15-07-11 09:18 AM, Doug Evans wrote:
> On Tue, Jul 7, 2015 at 11:15 AM, Pedro Alves <palves@redhat.com> wrote:
>> ...
>> Or even rename it while at it:
>>
>> void
>> peel_typedefs (struct type **type)
>> {
>> *type = check_typedef (*type);
>> }
>>
>> And so you'd write:
>>
>> > - CHECK_TYPEDEF (result);
>> > + peel_typedefs (&result);
>>
>> Then the code ends up self documenting, and there's no way to
>> forget to assign the return of the function back to the
>> argument.
>
> Hi.
>
> If we get into renaming, it would be really nice to fix another
> problem with check_typedefs.
> Many don't know (or forget) that it actually serves (at least) two
> main purposes.
> The first is the obvious removal of typedefs.
> The second is the resolution of opaque types.
> Forgetting the second purpose has caused bugs in the past,
> and just makes the code harder to read than it should be.
That's what I noticed when I was trying to get rid of check_typedef instances
where the return value is ignored. I am not comfortable with having a function
being used solely for its side-effects. That makes the calling code very unclear
about its intentions. Why not have a separate function that only serves the
second purpose? Perhaps that check_typedef could be refactored to make use of it,
I don't know.