[libiberty] add obstack macros (was Re: PR #53525 - track-macro-expansion performance regression)

[Dimitrios Apostolou <jimis at gmx dot net>]

Hi Dodji, I appreciate your review but I'm replying for now only for the 
libiberty.h part (attached updated patch), which I've needed elsewhere and 
seems the easiest to quickly apply.

On Wed, 18 Jul 2012, Dodji Seketeli wrote:
> > === modified file 'include/libiberty.h'
> > --- include/libiberty.h	2011-09-28 19:04:30 +0000
> > +++ include/libiberty.h	2012-07-07 16:04:01 +0000
>
> [...]
>
> > -/* Type-safe obstack allocator.  */
> > +/* Type-safe obstack allocator. You must first initialize the obstack with
> > +   obstack_init() or _obstack_begin(). */
>
> Why not just using the _obstack_begin function?  Why introducing the
> use of the obstack_init macro?

Please correct me if I'm wrong, but my impression is that obstack_init
is the documented way (see [1]), _obstack_begin seems hidden and gives 
access to dangerous (performance-wise) parameters, even though we always 
use it with the defaults (hmmm double checking I see that we mostly 
set size 0 which is default, but also alignment 0???).

[1] http://www.gnu.org/software/libc/manual/html_node/Preparing-for-Obstacks.html


> > -#define XOBFINISH(O, T)         ((T) obstack_finish ((O)))
>
> I think this change is not needed.  You basically remove this line to
> replace it with the hunk below, that comes later in the patch:
>
> > +#define XOBFINISH(O, PT)	((PT) obstack_finish ((O)))
>
> So I believe you could do away with the change.

I just wanted to point out that it expects and returns a Pointer to the 
Type, e.g. after you XOBGROW(O, int, DATA) you should XOBFINISH(O, int *).
It would probably be better for XOBFINISH to accept T and return (T *), 
but I didn't want to change existing status quo.

I think I addressed your other concerns (style, comments, unneeded macro 
params). CC'd maintainers. What do you think about attached patch?


Thanks,
Dimitris


P.S. Personally I prefer the original obstack interface, i.e. 
obstack_grow(), obstack_finish() etc. I just added the macros to follow 
existing style and avoid typecasting because of C++. Let me know if it's 
OK to use obstacks directly and I'll back out the patch.




2012-08-04 Dimitrios Apostolou <jimis@gmx.net>

	* libiberty.h
	(XOBDELETE,XOBGROW,XOBGROWVEC,XOBSHRINK,XOBSHRINKVEC): New
	type-safe macros for obstack allocation.
	(XOBFINISH): Renamed argument to PT since it is a pointer to T.

Attachment: libiberty.2.diff
Description: Text document