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

Simon Marchi
2018-04-26T02:58:14Z

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