Re: [PATCH 1/2] Introduce obstack_new, poison other "typed" obstack functions

[Simon Marchi <simon.marchi@polymtl.ca>]

Hi Tom,

Thanks for taking a look.

On 2018-04-25 18:36, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> This patch introduces a utility to make this pattern simpler:
> Simon>   foo *f = obstack_new<foo> ();
> 
> What about just having those types that can use this inherit from
> allocate_on_obstack?  Then you can write:
> 
>    foo *f = new (obstack) foo ();

I thought about this.  The downside I see is that it forces new to 
allocate on an obstack.  What if you want to use the standard new for 
the same type in another context?  Maybe this doesn't happen in practice 
though.

> It would be nice if we could have a global operator new that does this,
> but I don't think it is possible to have one that is limited to
> is_trivially_constructible classes.

That goes beyond my C++ knowledge.

> Maybe is_trivially_destructible is also needed somehow since an obstack
> isn't going to run those destructors.  Not sure how to manage this
> either, right now allocate_on_obstack just assumes you know what you're
> up to.

I remember you having some back and forth with Yao about this.  Here's 
my take on it.  It is possible to allocate objects that need to be 
destroyed on obstacks, the important thing is to call the destructor 
manually at some point before the obstack is freed.  It's done for 
mapped_index right now, for example.  You just can't allocate such an 
object on an obstack and forget about it, assuming it will be freed by 
magic (it's just like when you "new" something, you have to keep track 
of it to destroy it at some point).  When doing this, you obviously lose 
one nice feature of the obstack, not having to care about deallocation.  
So if that was the main reason an obstack is used, you might as well 
reconsider using an obstack at all [1].  But if the obstack is used for 
the more efficient memory allocation, then it would still make sense to 
use an obstack and track the objects separately so that they can be 
destroyed properly.  What we need is (not sure this is realistic): 
everywhere we allocate an object on an obstack and don't keep track of 
it for further destruction, have a static_assert there that the type 
should be trivially destructible.  If you happen to make said type non 
trivially destructible, you'll get an error and realize "oh right, the 
objects created here are never destroyed".

Or maybe there are better C++ ways of getting the same advantages as 
with an obstack (efficient allocation for a bunch of objects 
allocated/freed at the same time, data locality)?

[1] For example, is there a reason why mapped_index is allocated on the 
objfile obstack, and not newed just like mapped_debug_names?

Simon