Re: [PATCH] Remove CHECK_TYPEDEF, use check_typedef instead

[Pedro Alves <palves@redhat.com>]

On 07/06/2015 09:05 PM, Simon Marchi wrote:
> I think that the CHECK_TYPEDEF macro is not necessary, and even a bit
> annoying.  It makes unclear the fact that the "type" variables gets
> overwritten.  It has actually bitten me a few times.  I think the
> following, explicit form, is better.
> 
>   type = check_typedef (type);
> 

I don't have a strong opinion either.  Whatever's fine with others
is fine with me.

Playing devil's advocate, the CHECK_TYPEDEF macro has the advantage
that makes it clear that you want to peel away typedefs are don't
really care about the original type.

I'd argue that the real issue with the macro is that it takes the
type pointer argument "by non-const reference to pointer".

Another solution would be to make it a function/macro that
instead takes a pointer to a type pointer.  Something like:

 void
 CHECK_TYPEDEFS (struct type **type)
 {
   *type = check_typedef (*type);
 }

Then you'd write:

 > -	  CHECK_TYPEDEF (result);
 > +	  CHECK_TYPEDEF (&result);

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.

Thanks,
Pedro Alves