[Gc] Round 3 or 4 of C++ throw/nothrow patch.

Talbot, George Gtalbot at ansarisbio.com
Thu Dec 17 11:53:11 PST 2009


Revised patch attached, see responses inline.

--
George T. Talbot
<gtalbot at locuspharma.com>

> -----Original Message-----
> From: gc-bounces at napali.hpl.hp.com [mailto:gc-bounces at napali.hpl.hp.com]
> On Behalf Of Ivan Maidanski
> Sent: Thursday, December 17, 2009 12:23 PM
> To: gc at napali.hpl.hp.com
> Cc: Talbot, George
> Subject: Re: [Gc] Round 3 or 4 of C++ throw/nothrow patch.
>
> Hi!

[deleted my original email]

>
> My miscellaneous comment about the patch:
> 1. please use "-ru[N]" option for diff.

Done.  See attached.

> 2. please wrap long cod lines (longr than 78 chars).

Fixed.

> 3. cxxabi.h seems to be missing in some compiler suites (so, I can't
> compile on VC++, right?) - at least non-portable things should be guarded
> with the config macro;

They were for something else and no longer needed.  Took them out.  Thanks for catching this.

> 4. as for gc_cpp_init I think it's better to remove GC_INIT() (let it be
> the client responsiblity to call it) and rename it to something like
> GC_setup_cpp_oom().

Done.

> 5. Since we are really having only main trunc at BDWGC CVS, only bugfix
> patches are applied up to the final 7.2 release.

Would this qualify as an exception if it doesn't change the interface or default behavior?  I've been trying to get this in for a while...

> 6. the "__GNUC__ < 2 || __GNUC__ == 2..." in gc_cpp.cc has proper
> indentation (unlike that in your patch - as I recall I've already pointed
> to it in your previous patch). Also please clean up a bit your patch and
> remove the "no-change" fragments like:
>
> <     A* a = new A;       // a is collectable.
> ---
> >     A* a = new A;       // a is collectable.[spaces here]

Fixed.

> 7. Optionally it might be good (but I don't have a strong opinion here) to
> have the possibility of choosing between GC_MALLOC and
> GC_MALLOC_UNCOLLECTABLE from the client code at start-up (e.g export
> GC_set_new_collectible(0/1)).

If I recall correctly, Hans Boehm was worried about the overhead, so that's why I did this as a compile option.  I'm not averse to changing this to runtime-chooseable if necessary, though it'd be hard to call GC_set_new_collectible() before all constructors of static objects run.

> 8. gc_cpp_oom_fn() should be tagged with GC_CALLBACK.

Done.

> 9. I think newly-added exportable functions should have "GC_" prefix
> (instead of "gc_").

Done.

> I'm intentionally not commenting the functionality of the added feature -
> Hans knows better (the dark sides of the C++ part of GC).

I'm hoping with fingers crossed that this is non-intrusive enough...

Thanks for taking the time to look at the patch.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: cpp-throw.patch
Type: application/octet-stream
Size: 19053 bytes
Desc: cpp-throw.patch
Url : http://napali.hpl.hp.com/pipermail/gc/attachments/20091217/2471bf53/cpp-throw-0001.obj


More information about the Gc mailing list