Re: C++ PATCH: PR 21687

[Mark Mitchell <mark at codesourcery dot com>]

Daniel Jacobowitz wrote:
> On Fri, Sep 02, 2005 at 11:52:01AM -0700, Mark Mitchell wrote:
>
> > Daniel Berlin wrote:
> >
> > > On Fri, 2005-09-02 at 11:31 -0700, Mark Mitchell wrote:
> > >
> > >
> > > > We were missing ggc_push_context calls around functions in local
> > > > classes; we want to collect within those functions, but we have local
> > > > state that is not in GC roots.
> >
> > > Is there some reason we've chosen not to do the same here?
> >
> > Because we don't know what the local state is.  We're deep in the midst 
> > of parsing one function, and now enter another.  The other choice is to 
> > eliminate the ggc_collect call in expand_or_defer_fn in cp/semantics.c 
> > when in a local class, i.e, if function_depth.  That will result in 
> > potentially piling up a lot of garbage, but it should work I guess.
>
>
> That's what we used to do when compiling nested functions, yes.

Well, that seems bad -- but the zone allocator seems good, so I guess we 
can make that tradeoff.

> Because extracting all of its tendrils from the page allocator was very
> difficult and we were well into the 4.0 release cycle at the time.  I
> didn't want to remove the functions without removing the rest of the
> now-dead bits; if we do that, it'll be years before someone gets around
> to it.

I don't think that's a sufficient justification.  You did nothing to 
prevent people from reintroducing the calls, so I did reintroduce them. 
 As a result, we both lost: I'm going to have to redo the patch, and we 
had to have this conversation.  At the very least, the names should have 
been poisoned, or the functions given internal linkage, or renamed to 
ggc_dont_use_push_context(), or something.

After I check in a revised version, please check in changes to remove 
the entry points.  You can create a PR to remove the tendrils, if you 
like.  I'm much more concerned about losing developer time than I am 
about the cost of a few extra unused lines of code in ggc-page.c, 
especially if we're going to be switching to ggc-zone.c by default!

Thanks,

--
Mark Mitchell
CodeSourcery, LLC
mark@codesourcery.com
(916) 791-8304