Re: [PATCH] Remove CHECK_TYPEDEF, use check_typedef instead

[Pedro Alves <palves at redhat dot com>]

On 07/07/2015 09:15 PM, Simon Marchi wrote:

> That's a bit better, but I don't think there's an advantage here of
> having two ways to do the same thing.  If anything, it's confusing for
> new contributors.

OK.

> 
> I was going to suggest adding __attribute__((warn_unused_result)) to
> check_typedef, as it would also prevent forgetting assigning the result.
> Especially if we change it, people used to the old macro would be at
> risk of forgetting it.  However, I realized that at a few places the
> result of check_typedef is ignored.  It is used only to initialize the
> length field of the typedef, so that TYPE_LENGTH (the_typedef) will
> return the right thing [1].
> 
> Using a side-effect of check_typedef to get the length of the type right
> seems very hackish and error-prone (easy to forget something or to break
> something when moving code around).
> 
> I think there should be a get_type_length [2] function that returns what
> you would expect: the actual length of the type, after having peeled all
> layers of typedef. You wouldn't need to call check_typedef beforehand.

I'd be fine with it.

> It would be cleaner and safer, and would allow us to add warn_unused_result
> to check_typedef.  Because there should be no reason to call check_typedef
> than to obtain the resolved type.
> 
> Thoughts?
> 
> [1] For example: https://github.com/simark/binutils-gdb/blob/master/gdb/tracepoint.c#L1532
> [2] I can see a get_type_length function mentioned in the ChangeLog here:
> 
>       https://github.com/simark/binutils-gdb/blob/master/gdb/ChangeLog-2014#L9750
> 
>     but I can't find any trace of it in the source code (even when checkout out
>     that commit). Any idea? Joel perhaps?

Looks like it was present in earlier revisions of the patch, but later dropped:
Gooling for '"get_type_length" sanimir' found it:

  https://sourceware.org/ml/gdb-patches/2013-12/msg00127.html

Thanks,
Pedro Alves