Re: [PATCH] Remove CHECK_TYPEDEF, use check_typedef instead

[Simon Marchi <simon.marchi@ericsson.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.