Le 24/02/12 15:57, Brett Lentz a écrit :
Hi folks!
It's been a few months since I last mailed the list. I've been working on bringing some forked changes from the Passenger project into Boost due to their more general applicability.
With some help, I've got a working patch against SVN trunk that enables one of Passenger's more useful features: obtaining exception stacktraces all the way through the stack.
However, due to the size of the patch and some namespacing choices made by the Phusion people, I'd like to get some feedback on this patch to ensure it's suitable for being brought into Boost.
Hi, I wanted just to note that this patch concerns the libraries Boost.Exception, Boost.System and Boost.Thread and that it is a library without a name. Could it be possible to define these functionalities in independent libraries?
First, there's some helper classes that Phusion has put in an "oxt" namespace that I'm not really sure where they should be put or if suitable analogues already exist within Boost. For the current version of the patch, these files are simply put into 'boost/oxt' until I have a better location for them. Which libraries are using it in your patch. Maybe you can put it on boost::detail::oxt?
Second, the current patch uses boost::tracable_exception. I'd like to confirm whether that is acceptable or whether I should move this to a different location in the boost namespace.
Finally, any other comments about this patch are welcome. I have attached both the patch, and some example code that will demonstrate the functionality.
I don't know what is your intention proposing this patch, but I don't think the patch in its actual form could be applied to Boost. Its is a big patch changing a lot of code on existing libraries without any test, neither documentation. * Here there are the files contained on this patch. NEW Index: boost/thread/tracable_thread.hpp Index: boost/thread/detail/thread.hpp Index: boost/thread/exceptions.hpp NEW Index: boost/thread/dynamic_thread_group.hpp NEW Index: boost/oxt/detail/spin_lock_portable.hpp NEW Index: boost/oxt/detail/spin_lock_pthreads.hpp NEW Index: boost/oxt/detail/spin_lock_gcc_x86.hpp NEW Index: boost/oxt/detail/spin_lock_darwin.hpp NEW Index: boost/oxt/spin_lock.hppIndex: boost/oxt/macros.hpp NEW Index: boost/system/system_calls.hpp NEW Index: boost/exception/tracable_exception.hpp NEW Index: boost/exception/detail/backtrace_enabled.hpp NEW Index: boost/exception/detail/backtrace_disabled.hpp NEW Index: boost/exception/detail/tracable_exception_disabled.hpp NEW Index: boost/exception/detail/tracable_exception_enabled.hpp NEW Index: boost/exception/backtrace.hpp Index: libs/thread/build/Jamfile.v2 Index: libs/thread/src/tss_null.cpp NEW Index: libs/thread/src/pthread/backtrace.cpp Index: libs/thread/src/pthread/once.cpp NEW Index: libs/thread/src/pthread/tracable_thread.cpp Index: libs/thread/src/pthread/thread.cpp NEW Index: libs/thread/src/pthread/tracable_exception.cpp Index: libs/system/build/Jamfile.v2 NEW Index: libs/system/src/system_calls.cpp * I don't see any changes on windows files. Have you an implementation for windows? Note that the patch contains the files Index: boost/exception/tracable_exception.hpp Index: libs/thread/src/pthread/tracable_thread.cpp That is, the declaration is in Boost.Exception, while the implementation is in Boost.Thread :( * Is it necessary that tracable_thread inherits from thread? What about adding a tracker that can be used on any thread? * Why thread_exception don't inherits now from boost::system_error? This breaks the current exception interface. Is it absolutely needed that thread_exception inherits from tracable_exception to provide tracable exceptions? * It seems to me that dynamic_thread_group could be a nice to have, but not required to have backtraces, so it should be proposed separately. * There is already a spin lock implementation in Boost detail. Have you see if it responds to your needs? * I propose you to make a concrete proposal for the macros LIKELY and UNLIKELY to Boost.Config. * system_calls seems to me an independent thing of backtraces and tracable exceptions and quite dependant on posix interface. * It is current usage on Boost to place the implementation details in the namespace detail (see all the functions prefixed by "_" and to prefix any macro with BOOST_LIBNAME_. * I don't know if the license is compatible with the Boost license. Could you use the Boost license? * Please could you make a ticket concerning the changes in Index: libs/thread/src/tss_null.cpp Index: libs/thread/src/pthread/once.cpp Index: libs/thread/src/pthread/thread.cpp HTH, Vicente