Re: [Gc] Round 3 or 4 of C++ throw/nothrow patch.
ivmai at mail.ru
Thu Dec 17 09:23:20 PST 2009
"Talbot, George" <Gtalbot at ansarisbio.com> wrote:
> Hi all,
> I think I've addressed the issues with this patch that you guys have told me about in the past. This patch, when applied, does the following:
> 1) Adds both throwing and non-throwing versions of operator ::new to gc_cpp.h, with some code implementing these in gc_cpp.cc. The throwing versions of these throw std::bad_alloc when the collector can't satisfy a memory allocation request, as per the C++ standard. The non-throwing versions return 0 when the collector can't satisfy a memory allocation request, also as per the standard.
> 2) Adds a new function, gc_cpp_init(), which initializes the collector via GC_INIT() and installs an out-of-memory function that throws std::bad_alloc.
> 3) Adds a new configuration option --enable-new-collectable, which when given to configure defines GC_NEW_COLLECTABLE in include/private/config.h. This symbol, when defined, causes the implementations of the default operator ::new to use GC_MALLOC instead of GC_MALLOC_UNCOLLECTABLE. This option is very useful in programs that use the Standard C++ Library (assuming the allocator passes through memory requests to operator ::new as the GNU Libstdc++ does) so that memory allocated via the Standard C++ Library becomes collectable without having to supply a custom allocator.
> Programs and installations that don't call gc_cpp_init() and don't use the --enable-new-collectable configure option will be fully source compatible, and the behavior of the collector will be unchanged.
> I believe (hope) that I've addressed the concerns that folks previously had with this patch. Let me know if it looks OK to you. If it does, can it be applied?
My miscellaneous comment about the patch:
1. please use "-ru[N]" option for diff.
2. please wrap long cod lines (longr than 78 chars).
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;
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().
5. Since we are really having only main trunc at BDWGC CVS, only bugfix patches are applied up to the final 7.2 release.
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]
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)).
8. gc_cpp_oom_fn() should be tagged with GC_CALLBACK.
9. I think newly-added exportable functions should have "GC_" prefix (instead of "gc_").
I'm intentionally not commenting the functionality of the added feature - Hans knows better (the dark sides of the C++ part of GC).
> George T. Talbot
> <gtalbot at locuspharma.com>
> ATTACHMENT: application/x-patch (cpp-throw.patch)
More information about the Gc